Add an option for False Start without ALPN.

We can probably do this globally at this point since the cipher
requirements are much more restrict than they were in the beginning.
(Firefox, in particular, has done so far a while.) For now add a flag
since some consumer wanted this.

I'll see about connecting it to a Chrome field trial after our breakage
budget is no longer reserved for TLS 1.3.

Change-Id: Ib61dd5aae2dfd48b56e79873a7f3061a7631a5f8
Reviewed-on: https://boringssl-review.googlesource.com/23725
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index cdf68a1..c5b95e9 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3477,6 +3477,12 @@
 // connections which resumed a session.
 OPENSSL_EXPORT int32_t SSL_get_ticket_age_skew(const SSL *ssl);
 
+// SSL_CTX_set_false_start_allowed_without_alpn configures whether connections
+// on |ctx| may use False Start (if |SSL_MODE_ENABLE_FALSE_START| is enabled)
+// without negotiating ALPN.
+OPENSSL_EXPORT void SSL_CTX_set_false_start_allowed_without_alpn(SSL_CTX *ctx,
+                                                                 int allowed);
+
 
 // Deprecated functions.
 
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 53adba6..b801e82 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -1512,13 +1512,25 @@
 static bool can_false_start(const SSL_HANDSHAKE *hs) {
   SSL *const ssl = hs->ssl;
 
-  // False Start only for TLS 1.2 with an ECDHE+AEAD cipher and ALPN or NPN.
-  return !SSL_is_dtls(ssl) &&
-         SSL_version(ssl) == TLS1_2_VERSION &&
-         (!ssl->s3->alpn_selected.empty() ||
-          !ssl->s3->next_proto_negotiated.empty()) &&
-         hs->new_cipher->algorithm_mkey == SSL_kECDHE &&
-         hs->new_cipher->algorithm_mac == SSL_AEAD;
+  // False Start only for TLS 1.2 with an ECDHE+AEAD cipher.
+  if (SSL_is_dtls(ssl) ||
+      SSL_version(ssl) != TLS1_2_VERSION ||
+      hs->new_cipher->algorithm_mkey != SSL_kECDHE ||
+      hs->new_cipher->algorithm_mac != SSL_AEAD) {
+    return false;
+  }
+
+  // Additionally require ALPN or NPN by default.
+  //
+  // TODO(davidben): Can this constraint be relaxed globally now that cipher
+  // suite requirements have been relaxed?
+  if (!ssl->ctx->false_start_allowed_without_alpn &&
+      ssl->s3->alpn_selected.empty() &&
+      ssl->s3->next_proto_negotiated.empty()) {
+    return false;
+  }
+
+  return true;
 }
 
 static enum ssl_hs_wait_t do_finish_flight(SSL_HANDSHAKE *hs) {
diff --git a/ssl/internal.h b/ssl/internal.h
index 6c14383..4bef358 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2165,21 +2165,24 @@
   // If true, a client will request certificate timestamps.
   bool signed_cert_timestamps_enabled:1;
 
-  // tlsext_channel_id_enabled is one if Channel ID is enabled and zero
-  // otherwise. For a server, means that we'll accept Channel IDs from clients.
-  // For a client, means that we'll advertise support.
+  // tlsext_channel_id_enabled is whether Channel ID is enabled. For a server,
+  // means that we'll accept Channel IDs from clients.  For a client, means that
+  // we'll advertise support.
   bool tlsext_channel_id_enabled:1;
 
-  // grease_enabled is one if draft-davidben-tls-grease-01 is enabled and zero
-  // otherwise.
+  // grease_enabled is whether draft-davidben-tls-grease-01 is enabled.
   bool grease_enabled:1;
 
-  // allow_unknown_alpn_protos is one if the client allows unsolicited ALPN
+  // allow_unknown_alpn_protos is whether the client allows unsolicited ALPN
   // protocols from the peer.
   bool allow_unknown_alpn_protos:1;
 
-  // ed25519_enabled is one if Ed25519 is advertised in the handshake.
+  // ed25519_enabled is whether Ed25519 is advertised in the handshake.
   bool ed25519_enabled:1;
+
+  // false_start_allowed_without_alpn is whether False Start (if
+  // |SSL_MODE_ENABLE_FALSE_START| is enabled) is allowed without ALPN.
+  bool false_start_allowed_without_alpn:1;
 };
 
 // An ssl_shutdown_t describes the shutdown state of one end of the connection,
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 122313f..6da081e 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -2567,6 +2567,10 @@
   return ssl->s3->ticket_age_skew;
 }
 
+void SSL_CTX_set_false_start_allowed_without_alpn(SSL_CTX *ctx, int allowed) {
+  ctx->false_start_allowed_without_alpn = !!allowed;
+}
+
 int SSL_clear(SSL *ssl) {
   // In OpenSSL, reusing a client |SSL| with |SSL_clear| causes the previously
   // established session to be offered the next time around. wpa_supplicant
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 8acabd7..1f43be6 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1304,6 +1304,10 @@
 
   SSL_CTX_set_msg_callback(ssl_ctx.get(), MessageCallback);
 
+  if (config->allow_false_start_without_alpn) {
+    SSL_CTX_set_false_start_allowed_without_alpn(ssl_ctx.get(), 1);
+  }
+
   if (old_ctx) {
     uint8_t keys[48];
     if (!SSL_CTX_get_tlsext_ticket_keys(old_ctx, &keys, sizeof(keys)) ||
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 66c13fc..a6b1701 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2247,6 +2247,21 @@
 			expectedLocalError: "tls: peer did not false start: EOF",
 		},
 		{
+			name: "FalseStart-NoALPNAllowed",
+			config: Config{
+				MaxVersion:   VersionTLS12,
+				CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+				Bugs: ProtocolBugs{
+					ExpectFalseStart: true,
+				},
+			},
+			flags: []string{
+				"-false-start",
+				"-allow-false-start-without-alpn",
+			},
+			shimWritesFirst: true,
+		},
+		{
 			name: "NoFalseStart-NoAEAD",
 			config: Config{
 				MaxVersion:   VersionTLS12,
@@ -4831,8 +4846,6 @@
 			expectedNextProtoType: npn,
 		})
 
-		// TODO(davidben): Add tests for when False Start doesn't trigger.
-
 		// Client does False Start and negotiates NPN.
 		tests = append(tests, testCase{
 			name: "FalseStart",
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index a5ce5a1..efe5acb 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -128,6 +128,8 @@
   { "-allow-unknown-alpn-protos", &TestConfig::allow_unknown_alpn_protos },
   { "-enable-ed25519", &TestConfig::enable_ed25519 },
   { "-use-custom-verify-callback", &TestConfig::use_custom_verify_callback },
+  { "-allow-false-start-without-alpn",
+    &TestConfig::allow_false_start_without_alpn },
 };
 
 const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index ea12d34..783471a 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -144,6 +144,7 @@
   bool enable_ed25519 = false;
   bool use_custom_verify_callback = false;
   std::string expect_msg_callback;
+  bool allow_false_start_without_alpn = false;
 };
 
 bool ParseConfig(int argc, char **argv, TestConfig *out_initial,