Skipping early data on 0RTT rejection.
BUG=101
Change-Id: Ia1edbccee535b0bc3a0e18465286d5bcca240035
Reviewed-on: https://boringssl-review.googlesource.com/12470
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>
diff --git a/ssl/internal.h b/ssl/internal.h
index 1d78ec1..5893d4d 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1393,6 +1393,10 @@
* completed. */
unsigned initial_handshake_complete:1;
+ /* skip_early_data instructs the record layer to skip unexpected early data
+ * messages when 0RTT is rejected. */
+ unsigned skip_early_data:1;
+
/* read_buffer holds data from the transport to be processed. */
SSL3_BUFFER read_buffer;
/* write_buffer holds data to be written to the transport. */
@@ -1423,6 +1427,10 @@
/* recv_shutdown is the shutdown state for the send half of the connection. */
enum ssl_shutdown_t send_shutdown;
+ /* early_data_skipped is the amount of early data that has been skipped by the
+ * record layer. */
+ uint16_t early_data_skipped;
+
int alert_dispatch;
uint8_t send_alert[2];
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 421232f..21005f0 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2034,6 +2034,7 @@
/* Pre-Shared Key Exchange Modes
*
* https://tools.ietf.org/html/draft-ietf-tls-tls13-18#section-4.2.7 */
+
static int ext_psk_key_exchange_modes_add_clienthello(SSL *ssl, CBB *out) {
uint16_t min_version, max_version;
if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
@@ -2078,6 +2079,35 @@
}
+/* Early Data Indication
+ *
+ * https://tools.ietf.org/html/draft-ietf-tls-tls13-18#section-4.2.8 */
+
+static int ext_early_data_add_clienthello(SSL *ssl, CBB *out) {
+ /* TODO(svaldez): Support 0RTT. */
+ return 1;
+}
+
+static int ext_early_data_parse_clienthello(SSL *ssl, uint8_t *out_alert,
+ CBS *contents) {
+ if (contents == NULL) {
+ return 1;
+ }
+
+ if (CBS_len(contents) != 0) {
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return 0;
+ }
+
+ /* Since we don't currently accept 0-RTT, we have to skip past any early data
+ * the client might have sent. */
+ if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) {
+ ssl->s3->skip_early_data = 1;
+ }
+ return 1;
+}
+
+
/* Key Share
*
* https://tools.ietf.org/html/draft-ietf-tls-tls13-16#section-4.2.5 */
@@ -2561,6 +2591,14 @@
dont_add_serverhello,
},
{
+ TLSEXT_TYPE_early_data,
+ NULL,
+ ext_early_data_add_clienthello,
+ forbid_parse_serverhello,
+ ext_early_data_parse_clienthello,
+ dont_add_serverhello,
+ },
+ {
TLSEXT_TYPE_supported_versions,
NULL,
ext_supported_versions_add_clienthello,
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index a6496fd..caa9cde 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -1109,6 +1109,25 @@
// copies of each KeyShareEntry.
DuplicateKeyShares bool
+ // SendEarlyAlert, if true, sends a fatal alert after the ClientHello.
+ SendEarlyAlert bool
+
+ // SendEarlyDataLength, if non-zero, is the amount of early data to send after
+ // the ClientHello.
+ SendEarlyDataLength int
+
+ // OmitEarlyDataExtension, if true, causes the early data extension to
+ // be omitted in the ClientHello.
+ OmitEarlyDataExtension bool
+
+ // SendEarlyDataOnSecondClientHello, if true, causes the TLS 1.3 client to
+ // send early data after the second ClientHello.
+ SendEarlyDataOnSecondClientHello bool
+
+ // InterleaveEarlyData, if true, causes the TLS 1.3 client to send early
+ // data interleaved with the second ClientHello and the client Finished.
+ InterleaveEarlyData bool
+
// EmptyEncryptedExtensions, if true, causes the TLS 1.3 server to
// emit an empty EncryptedExtensions block.
EmptyEncryptedExtensions bool
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 39c2785..80a5b06 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -1768,3 +1768,16 @@
c.out.doKeyUpdate(c, true)
return nil
}
+
+func (c *Conn) sendFakeEarlyData(len int) error {
+ // Assemble a fake early data record. This does not use writeRecord
+ // because the record layer may be using different keys at this point.
+ payload := make([]byte, 5+len)
+ payload[0] = byte(recordTypeApplicationData)
+ payload[1] = 3
+ payload[2] = 1
+ payload[3] = byte(len >> 8)
+ payload[4] = byte(len)
+ _, err := c.conn.Write(payload)
+ return err
+}
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index d074778..2ebd0dc 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -319,6 +319,10 @@
hello.cipherSuites = c.config.Bugs.SendCipherSuites
}
+ if c.config.Bugs.SendEarlyDataLength > 0 && !c.config.Bugs.OmitEarlyDataExtension {
+ hello.hasEarlyData = true
+ }
+
var helloBytes []byte
if c.config.Bugs.SendV2ClientHello {
// Test that the peer left-pads random.
@@ -356,6 +360,12 @@
if err := c.simulatePacketLoss(nil); err != nil {
return err
}
+ if c.config.Bugs.SendEarlyAlert {
+ c.sendAlert(alertHandshakeFailure)
+ }
+ if c.config.Bugs.SendEarlyDataLength > 0 {
+ c.sendFakeEarlyData(c.config.Bugs.SendEarlyDataLength)
+ }
msg, err := c.readHandshake()
if err != nil {
return err
@@ -452,16 +462,28 @@
hello.hasKeyShares = false
}
- hello.hasEarlyData = false
+ hello.hasEarlyData = c.config.Bugs.SendEarlyDataOnSecondClientHello
hello.raw = nil
if len(hello.pskIdentities) > 0 {
generatePSKBinders(hello, pskCipherSuite, session.masterSecret, append(helloBytes, helloRetryRequest.marshal()...), c.config)
}
secondHelloBytes = hello.marshal()
- c.writeRecord(recordTypeHandshake, secondHelloBytes)
+
+ if c.config.Bugs.InterleaveEarlyData {
+ c.sendFakeEarlyData(4)
+ c.writeRecord(recordTypeHandshake, secondHelloBytes[:16])
+ c.sendFakeEarlyData(4)
+ c.writeRecord(recordTypeHandshake, secondHelloBytes[16:])
+ } else {
+ c.writeRecord(recordTypeHandshake, secondHelloBytes)
+ }
c.flushHandshake()
+ if c.config.Bugs.SendEarlyDataOnSecondClientHello {
+ c.sendFakeEarlyData(4)
+ }
+
msg, err = c.readHandshake()
if err != nil {
return err
@@ -871,6 +893,12 @@
if c.config.Bugs.PartialClientFinishedWithClientHello {
// The first byte has already been sent.
c.writeRecord(recordTypeHandshake, finished.marshal()[1:])
+ } else if c.config.Bugs.InterleaveEarlyData {
+ finishedBytes := finished.marshal()
+ c.sendFakeEarlyData(4)
+ c.writeRecord(recordTypeHandshake, finishedBytes[:1])
+ c.sendFakeEarlyData(4)
+ c.writeRecord(recordTypeHandshake, finishedBytes[1:])
} else {
c.writeRecord(recordTypeHandshake, finished.marshal())
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 683f07c..35742cb 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5568,7 +5568,7 @@
"-signed-cert-timestamps",
base64.StdEncoding.EncodeToString([]byte{0, 0}),
},
- shouldFail: true,
+ shouldFail: true,
expectedError: ":INVALID_SCT_LIST:",
})
}
@@ -9033,6 +9033,144 @@
})
testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyDataLength: 4,
+ },
+ },
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData-OmitEarlyDataExtension",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyDataLength: 4,
+ OmitEarlyDataExtension: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:",
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData-TooMuchData",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyDataLength: 16384 + 1,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":TOO_MUCH_SKIPPED_EARLY_DATA:",
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData-Interleaved",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyDataLength: 4,
+ InterleaveEarlyData: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:",
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData-EarlyDataInTLS12",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyDataLength: 4,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":UNEXPECTED_RECORD:",
+ flags: []string{"-max-version", strconv.Itoa(VersionTLS12)},
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData-HRR",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyDataLength: 4,
+ },
+ DefaultCurves: []CurveID{},
+ },
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData-HRR-Interleaved",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyDataLength: 4,
+ InterleaveEarlyData: true,
+ },
+ DefaultCurves: []CurveID{},
+ },
+ shouldFail: true,
+ expectedError: ":UNEXPECTED_RECORD:",
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData-HRR-TooMuchData",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyDataLength: 16384 + 1,
+ },
+ DefaultCurves: []CurveID{},
+ },
+ shouldFail: true,
+ expectedError: ":TOO_MUCH_SKIPPED_EARLY_DATA:",
+ })
+
+ // Test that skipping early data looking for cleartext correctly
+ // processes an alert record.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData-HRR-FatalAlert",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyAlert: true,
+ SendEarlyDataLength: 4,
+ },
+ DefaultCurves: []CurveID{},
+ },
+ shouldFail: true,
+ expectedError: ":SSLV3_ALERT_HANDSHAKE_FAILURE:",
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipEarlyData-SecondClientHelloEarlyData",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ SendEarlyDataOnSecondClientHello: true,
+ },
+ DefaultCurves: []CurveID{},
+ },
+ shouldFail: true,
+ expectedLocalError: "remote error: bad record MAC",
+ })
+
+ testCases = append(testCases, testCase{
testType: clientTest,
name: "EmptyEncryptedExtensions",
config: Config{
diff --git a/ssl/tls_record.c b/ssl/tls_record.c
index 5931922..c52909c 100644
--- a/ssl/tls_record.c
+++ b/ssl/tls_record.c
@@ -125,6 +125,12 @@
* forever. */
static const uint8_t kMaxEmptyRecords = 32;
+/* kMaxEarlyDataSkipped is the maximum amount of data processed when skipping
+ * over early data. Without this limit an attacker could send records at a
+ * faster rate than we can process and cause trial decryption to loop
+ * forever. */
+static const size_t kMaxEarlyDataSkipped = 16384;
+
/* kMaxWarningAlerts is the number of consecutive warning alerts that will be
* processed. */
static const uint8_t kMaxWarningAlerts = 4;
@@ -246,15 +252,32 @@
ssl_do_msg_callback(ssl, 0 /* read */, SSL3_RT_HEADER, in,
SSL3_RT_HEADER_LENGTH);
+ *out_consumed = in_len - CBS_len(&cbs);
+
+ /* Skip early data received when expecting a second ClientHello if we rejected
+ * 0RTT. */
+ if (ssl->s3->skip_early_data &&
+ ssl->s3->aead_read_ctx == NULL &&
+ type == SSL3_RT_APPLICATION_DATA) {
+ goto skipped_data;
+ }
+
/* Decrypt the body in-place. */
if (!SSL_AEAD_CTX_open(ssl->s3->aead_read_ctx, out, type, version,
ssl->s3->read_sequence, (uint8_t *)CBS_data(&body),
CBS_len(&body))) {
+ if (ssl->s3->skip_early_data &&
+ ssl->s3->aead_read_ctx != NULL) {
+ ERR_clear_error();
+ goto skipped_data;
+ }
+
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
*out_alert = SSL_AD_BAD_RECORD_MAC;
return ssl_open_record_error;
}
- *out_consumed = in_len - CBS_len(&cbs);
+
+ ssl->s3->skip_early_data = 0;
if (!ssl_record_sequence_update(ssl->s3->read_sequence, 8)) {
*out_alert = SSL_AD_INTERNAL_ERROR;
@@ -310,6 +333,20 @@
*out_type = type;
return ssl_open_record_success;
+
+skipped_data:
+ ssl->s3->early_data_skipped += *out_consumed;
+ if (ssl->s3->early_data_skipped < *out_consumed) {
+ ssl->s3->early_data_skipped = kMaxEarlyDataSkipped + 1;
+ }
+
+ if (ssl->s3->early_data_skipped > kMaxEarlyDataSkipped) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_TOO_MUCH_SKIPPED_EARLY_DATA);
+ *out_alert = SSL_AD_UNEXPECTED_MESSAGE;
+ return ssl_open_record_error;
+ }
+
+ return ssl_open_record_discard;
}
static int do_seal_record(SSL *ssl, uint8_t *out, size_t *out_len,
