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>

crypto/err/ssl.errordata

@@ -170,6 +170,7 @@ 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,270,TOO_MUCH_SKIPPED_EARLY_DATA
 SSL,221,UNABLE_TO_FIND_ECDH_PARAMETERS
 SSL,222,UNEXPECTED_EXTENSION
 SSL,223,UNEXPECTED_MESSAGE

include/openssl/ssl.h

@@ -4552,6 +4552,7 @@ BORINGSSL_MAKE_DELETER(SSL_SESSION, SSL_SESSION_free)
 #define SSL_R_PRE_SHARED_KEY_MUST_BE_LAST 267
 #define SSL_R_OLD_SESSION_PRF_HASH_MISMATCH 268
 #define SSL_R_INVALID_SCT_LIST 269
+#define SSL_R_TOO_MUCH_SKIPPED_EARLY_DATA 270
 #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

ssl/internal.h

@@ -1393,6 +1393,10 @@ typedef struct ssl3_state_st {
    * 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 @@ typedef struct ssl3_state_st {
   /* 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];
 

ssl/t1_lib.c

@@ -2034,6 +2034,7 @@ int ssl_ext_pre_shared_key_add_serverhello(SSL *ssl, CBB *out) {
 /* 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 @@ static int ext_psk_key_exchange_modes_parse_clienthello(SSL *ssl,
 }
 
 
+/* 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 */
@@ -2560,6 +2590,14 @@ static const struct tls_extension kExtensions[] = {
     ext_psk_key_exchange_modes_parse_clienthello,
     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,

ssl/test/runner/common.go

@@ -1109,6 +1109,25 @@ type ProtocolBugs struct {
 	// 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

ssl/test/runner/conn.go

@@ -1768,3 +1768,16 @@ func (c *Conn) sendKeyUpdateLocked(keyUpdateRequest byte) error {
 	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
+}

ssl/test/runner/handshake_client.go

@@ -319,6 +319,10 @@ NextCipherSuite:
 		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 @@ NextCipherSuite:
 	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 @@ NextCipherSuite:
 			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 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 	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())
 	}

ssl/test/runner/runner.go

@@ -5568,7 +5568,7 @@ func addExtensionTests() {
 			"-signed-cert-timestamps",
 			base64.StdEncoding.EncodeToString([]byte{0, 0}),
 		},
-		shouldFail: true,
+		shouldFail:    true,
 		expectedError: ":INVALID_SCT_LIST:",
 	})
 }
@@ -9032,6 +9032,144 @@ func addTLS13HandshakeTests() {
 		expectedError: ":DUPLICATE_KEY_SHARE:",
 	})
 
+	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",

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 @@ enum ssl_open_record_t tls_open_record(SSL *ssl, uint8_t *out_type, CBS *out,
   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 @@ enum ssl_open_record_t tls_open_record(SSL *ssl, uint8_t *out_type, CBS *out,
 
   *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,