Bug 1134548 - Send the maximum version in ClientHello when attempting…

… resumption, r=wtc

--HG--
extra : rebase_source : 2beb046cf0aeb1e7e466d8b84b15cae723eddf7f
extra : amend_source : ef7c5760ba2bb499ec2402b3a6e8aee213bf5f6a

external_tests/ssl_gtest/ssl_loopback_unittest.cc

@@ -44,7 +44,7 @@ TEST_P(TlsConnectGeneric, SetupOnly) {}
 
 TEST_P(TlsConnectGeneric, Connect) {
   Connect();
-  client_->CheckVersion(SSL_LIBRARY_VERSION_TLS_1_2);
+  client_->CheckVersion(std::get<1>(GetParam()));
 }
 
 TEST_P(TlsConnectGeneric, ConnectResumed) {
@@ -146,6 +146,49 @@ TEST_P(TlsConnectGeneric, ConnectClientNoneServerBoth) {
   CheckResumption(RESUME_NONE);
 }
 
+TEST_P(TlsConnectGeneric, ConnectTLS_1_1_Only) {
+  EnsureTlsSetup();
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
+                           SSL_LIBRARY_VERSION_TLS_1_1);
+
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
+                           SSL_LIBRARY_VERSION_TLS_1_1);
+
+  Connect();
+
+  client_->CheckVersion(SSL_LIBRARY_VERSION_TLS_1_1);
+}
+
+TEST_P(TlsConnectGeneric, ConnectTLS_1_2_Only) {
+  EnsureTlsSetup();
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_2);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2,
+                           SSL_LIBRARY_VERSION_TLS_1_2);
+  Connect();
+  client_->CheckVersion(SSL_LIBRARY_VERSION_TLS_1_2);
+}
+
+TEST_P(TlsConnectGeneric, ResumeWithHigherVersion) {
+  EnsureTlsSetup();
+  ConfigureSessionCache(RESUME_SESSIONID, RESUME_SESSIONID);
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
+                           SSL_LIBRARY_VERSION_TLS_1_1);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
+                           SSL_LIBRARY_VERSION_TLS_1_1);
+  Connect();
+
+  Reset();
+  EnsureTlsSetup();
+  client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
+                           SSL_LIBRARY_VERSION_TLS_1_2);
+  server_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_1,
+                           SSL_LIBRARY_VERSION_TLS_1_2);
+  Connect();
+  CheckResumption(RESUME_NONE);
+  client_->CheckVersion(SSL_LIBRARY_VERSION_TLS_1_2);
+}
+
 TEST_P(TlsConnectGeneric, ConnectAlpn) {
   EnableAlpn();
   Connect();

lib/ssl/ssl3con.c

@@ -4987,23 +4987,17 @@ ssl3_SendClientHello(sslSocket *ss, PRBool resending)
 	    sidOK = PR_FALSE;
 	}
 
-	/* TLS 1.0 (RFC 2246) Appendix E says:
-	 *   Whenever a client already knows the highest protocol known to
-	 *   a server (for example, when resuming a session), it should
-	 *   initiate the connection in that native protocol.
-	 * So we pass sid->version to ssl3_NegotiateVersion() here, except
-	 * when renegotiating.
-	 *
-	 * Windows SChannel compares the client_version inside the RSA
-	 * EncryptedPreMasterSecret of a renegotiation with the
-	 * client_version of the initial ClientHello rather than the
-	 * ClientHello in the renegotiation. To work around this bug, we
-	 * continue to use the client_version used in the initial
-	 * ClientHello when renegotiating.
-	 */
 	if (sidOK) {
+            /* Set ss->version based on the session cache */
 	    if (ss->firstHsDone) {
 		/*
+	         * Windows SChannel compares the client_version inside the RSA
+	         * EncryptedPreMasterSecret of a renegotiation with the
+	         * client_version of the initial ClientHello rather than the
+	         * ClientHello in the renegotiation. To work around this bug, we
+	         * continue to use the client_version used in the initial
+	         * ClientHello when renegotiating.
+	         *
 		 * The client_version of the initial ClientHello is still
 		 * available in ss->clientHelloVersion. Ensure that
 		 * sid->version is bounded within
@@ -5017,10 +5011,22 @@ ssl3_SendClientHello(sslSocket *ss, PRBool resending)
 		    sidOK = PR_FALSE;
 		}
 	    } else {
-		if (ssl3_NegotiateVersion(ss, sid->version,
-					  PR_FALSE) != SECSuccess) {
+                /*
+                 * Check sid->version is OK first.
+                 * Previously, we would cap the version based on sid->version,
+                 * but that prevents negotiation of a higher version if the
+                 * previous session was reduced (e.g., with version fallback)
+                 */
+		if (sid->version < ss->vrange.min || 
+                    sid->version > ss->vrange.max) {
 		    sidOK = PR_FALSE;
-		}
+		} else {
+	            rv = ssl3_NegotiateVersion(ss, SSL_LIBRARY_VERSION_MAX_SUPPORTED,
+                                               PR_TRUE);
+	            if (rv != SECSuccess) {
+                        return rv;	/* error code was set */
+                    }
+	        }
 	    }
 	}