Bug 1462303 - Allow TLS 1.3 compat mode when attempting to resume TLS 1.2, r=ekr,ttaubert NSS_3_37_BRANCH
authorMartin Thomson <martin.thomson@gmail.com>
Fri, 18 May 2018 12:51:29 +1000
branchNSS_3_37_BRANCH
changeset 14358 2f5e8b4bf89729ea5ee18c089e684d4e8138b557
parent 14348 788ef5d1037c478e286856e98fe1210b6eec1b3a
child 14361 1e48d7c6fa61abe90388cb64c3ba86689cc565de
push id3090
push usermartin.thomson@gmail.com
push dateFri, 18 May 2018 23:03:19 +0000
reviewersekr, ttaubert
bugs1462303
Bug 1462303 - Allow TLS 1.3 compat mode when attempting to resume TLS 1.2, r=ekr,ttaubert
gtests/ssl_gtest/ssl_tls13compat_unittest.cc
lib/ssl/ssl3con.c
--- a/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
+++ b/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
@@ -431,9 +431,25 @@ TEST_F(TlsConnectDatagram13, CompatModeD
               server_records->record(i).header.content_type());
   }
 
   uint32_t session_id_len = 0;
   EXPECT_TRUE(server_hello->buffer().Read(2 + 32, 1, &session_id_len));
   EXPECT_EQ(0U, session_id_len);
 }
 
+TEST_F(Tls13CompatTest, ConnectWith12ThenAttemptToResume13CompatMode) {
+  ConfigureSessionCache(RESUME_SESSIONID, RESUME_SESSIONID);
+  ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_2);
+  Connect();
+
+  Reset();
+  ExpectResumption(RESUME_NONE);
+  version_ = SSL_LIBRARY_VERSION_TLS_1_3;
+  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_3);
+  EnableCompatMode();
+  Connect();
+}
+
 }  // namespace nss_test
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -6159,38 +6159,48 @@ ssl_ClientSetCipherSuite(sslSocket *ss, 
 
 /* Check that session ID we received from the server, if any, matches our
  * expectations, depending on whether we're in compat mode and whether we
  * negotiated TLS 1.3+ or TLS 1.2-.
  */
 static PRBool
 ssl_CheckServerSessionIdCorrectness(sslSocket *ss, SECItem *sidBytes)
 {
-    PRBool sid_match = PR_FALSE;
-    PRBool sent_fake_sid = ss->opt.enableTls13CompatMode && !IS_DTLS(ss);
-
-    /* If in compat mode and we received a session ID with the right length
-     * then compare it to the fake one we sent in the ClientHello. */
-    if (sent_fake_sid && sidBytes->len == SSL3_SESSIONID_BYTES) {
-        PRUint8 buf[SSL3_SESSIONID_BYTES];
-        ssl_MakeFakeSid(ss, buf);
-        sid_match = PORT_Memcmp(buf, sidBytes->data, sidBytes->len) == 0;
-    }
-
-    /* TLS 1.2: SessionID shouldn't match the fake one. */
+    sslSessionID *sid = ss->sec.ci.sid;
+    PRBool sidMatch = PR_FALSE;
+    PRBool sentFakeSid = PR_FALSE;
+    PRBool sentRealSid = sid && sid->version < SSL_LIBRARY_VERSION_TLS_1_3;
+
+    /* If attempting to resume a TLS 1.2 connection, the session ID won't be a
+     * fake. Check for the real value. */
+    if (sentRealSid) {
+        sidMatch = (sidBytes->len == sid->u.ssl3.sessionIDLength) &&
+                   PORT_Memcmp(sid->u.ssl3.sessionID, sidBytes->data, sidBytes->len) == 0;
+    } else {
+        /* Otherwise, the session ID was a fake if TLS 1.3 compat mode is
+         * enabled.  If so, check for the fake value. */
+        sentFakeSid = ss->opt.enableTls13CompatMode && !IS_DTLS(ss);
+        if (sentFakeSid && sidBytes->len == SSL3_SESSIONID_BYTES) {
+            PRUint8 buf[SSL3_SESSIONID_BYTES];
+            ssl_MakeFakeSid(ss, buf);
+            sidMatch = PORT_Memcmp(buf, sidBytes->data, sidBytes->len) == 0;
+        }
+    }
+
+    /* TLS 1.2: Session ID shouldn't match if we sent a fake. */
     if (ss->version < SSL_LIBRARY_VERSION_TLS_1_3) {
-        return !sid_match;
-    }
-
-    /* TLS 1.3: [Compat Mode] Session ID should match the fake one. */
-    if (sent_fake_sid) {
-        return sid_match;
-    }
-
-    /* TLS 1.3: [Non-Compat Mode] Server shouldn't send a session ID. */
+        return !sentFakeSid || !sidMatch;
+    }
+
+    /* TLS 1.3: We sent a session ID.  The server's should match. */
+    if (sentRealSid || sentFakeSid) {
+        return sidMatch;
+    }
+
+    /* TLS 1.3: The server shouldn't send a session ID. */
     return sidBytes->len == 0;
 }
 
 /* Called from ssl3_HandleHandshakeMessage() when it has deciphered a complete
  * ssl3 ServerHello message.
  * Caller must hold Handshake and RecvBuf locks.
  */
 static SECStatus