From b2e3f773e8a74fa17d096ad061cf9bc198a812ce Mon Sep 17 00:00:00 2001
From: EKR <ekr@rtfm.com>
Date: Thu, 11 Oct 2018 18:51:04 -0700
Subject: [PATCH] Bug 1498437 - Require that the server negotiate TLS 1.3 if we
 sent ESNI. r=mt

Reviewers: mt

Tags: #secure-revision

Bug #: 1498437

Differential Revision: https://phabricator.services.mozilla.com/D8496
---
 gtests/ssl_gtest/tls_esni_unittest.cc | 17 +++++++++++++++++
 lib/ssl/ssl3con.c                     | 15 +++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/gtests/ssl_gtest/tls_esni_unittest.cc b/gtests/ssl_gtest/tls_esni_unittest.cc
index 8619214ade..3c860a0b2b 100644
--- a/gtests/ssl_gtest/tls_esni_unittest.cc
+++ b/gtests/ssl_gtest/tls_esni_unittest.cc
@@ -450,4 +450,21 @@ TEST_P(TlsConnectTls13, ConnectBogusEsniExtensionEE) {
   ConnectExpectAlert(client_, illegal_parameter);
   client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_ESNI_EXTENSION);
 }
+
+// ESNI is a commitment to doing TLS 1.3 or above.
+// The TLS 1.2 server ignores ESNI and processes the dummy SNI.
+// The client then aborts when it sees the server did TLS 1.2.
+TEST_P(TlsConnectTls13, EsniButTLS12Server) {
+  EnsureTlsSetup();
+  SetupEsni(client_, server_);
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_3);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_2);
+  ConnectExpectAlert(client_, kTlsAlertProtocolVersion);
+  client_->CheckErrorCode(SSL_ERROR_UNSUPPORTED_VERSION);
+  server_->CheckErrorCode(SSL_ERROR_PROTOCOL_VERSION_ALERT);
+  ASSERT_FALSE(SSLInt_ExtensionNegotiated(server_->ssl_fd(),
+                                          ssl_tls13_encrypted_sni_xtn));
+}
 }
diff --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c
index e82ab09877..a44c846afd 100644
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -6563,9 +6563,20 @@ ssl3_HandleServerHello(sslSocket *ss, PRUint8 *b, PRUint32 length)
         goto alert_loser;
     }
 
-    /* The server didn't pick 1.3 although we either received a
-     * HelloRetryRequest, or we prepared to send early app data. */
+    /* There are three situations in which the server must pick
+     * TLS 1.3.
+     *
+     * 1. We offered ESNI.
+     * 2. We received HRR
+     * 3. We sent early app data.
+     *
+     */
     if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) {
+        if (ss->xtnData.esniPrivateKey) {
+            desc = protocol_version;
+            errCode = SSL_ERROR_UNSUPPORTED_VERSION;
+            goto alert_loser;
+        }
         if (isHelloRetry || ss->ssl3.hs.helloRetry) {
             /* SSL3_SendAlert() will uncache the SID. */
             desc = illegal_parameter;