From 75a70c2e55b178acb9f43731cc97a594a91a2651 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Mon, 15 Feb 2016 11:41:40 -0500
Subject: [PATCH] crypto/tls: Never resume sessions across different versions.

Instead, decline the session and do a full handshake. The semantics of
cross-version resume are unclear, and all major client implementations
treat this as a fatal error. (This doesn't come up very much, mostly if
the client does the browser version fallback without sharding the
session cache.)

See BoringSSL's bdf5e72f50e25f0e45e825c156168766d8442dde and OpenSSL's
9e189b9dc10786c755919e6792e923c584c918a1.

Change-Id: I51ca95ac1691870dd0c148fd967739e2d4f58824
Reviewed-on: https://go-review.googlesource.com/21152
Reviewed-by: Adam Langley <agl@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
---
 handshake_server.go      |  6 ++---
 handshake_server_test.go | 58 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/handshake_server.go b/handshake_server.go
index 8e94f21..cf617df 100644
--- a/handshake_server.go
+++ b/handshake_server.go
@@ -284,10 +284,8 @@ func (hs *serverHandshakeState) checkForResumption() bool {
 		return false
 	}
 
-	if hs.sessionState.vers > hs.clientHello.vers {
-		return false
-	}
-	if vers, ok := c.config.mutualVersion(hs.sessionState.vers); !ok || vers != hs.sessionState.vers {
+	// Never resume a session for a different TLS version.
+	if c.vers != hs.sessionState.vers {
 		return false
 	}
 
diff --git a/handshake_server_test.go b/handshake_server_test.go
index fba81f6..d878f99 100644
--- a/handshake_server_test.go
+++ b/handshake_server_test.go
@@ -399,6 +399,64 @@ func TestSCTHandshake(t *testing.T) {
 	}
 }
 
+func TestCrossVersionResume(t *testing.T) {
+	serverConfig := &Config{
+		CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA},
+		Certificates: testConfig.Certificates,
+	}
+	clientConfig := &Config{
+		CipherSuites:       []uint16{TLS_RSA_WITH_AES_128_CBC_SHA},
+		InsecureSkipVerify: true,
+		ClientSessionCache: NewLRUClientSessionCache(1),
+		ServerName:         "servername",
+	}
+
+	// Establish a session at TLS 1.1.
+	clientConfig.MaxVersion = VersionTLS11
+	_, _, err := testHandshake(clientConfig, serverConfig)
+	if err != nil {
+		t.Fatalf("handshake failed: %s", err)
+	}
+
+	// The client session cache now contains a TLS 1.1 session.
+	state, _, err := testHandshake(clientConfig, serverConfig)
+	if err != nil {
+		t.Fatalf("handshake failed: %s", err)
+	}
+	if !state.DidResume {
+		t.Fatalf("handshake did not resume at the same version")
+	}
+
+	// Test that the server will decline to resume at a lower version.
+	clientConfig.MaxVersion = VersionTLS10
+	state, _, err = testHandshake(clientConfig, serverConfig)
+	if err != nil {
+		t.Fatalf("handshake failed: %s", err)
+	}
+	if state.DidResume {
+		t.Fatalf("handshake resumed at a lower version")
+	}
+
+	// The client session cache now contains a TLS 1.0 session.
+	state, _, err = testHandshake(clientConfig, serverConfig)
+	if err != nil {
+		t.Fatalf("handshake failed: %s", err)
+	}
+	if !state.DidResume {
+		t.Fatalf("handshake did not resume at the same version")
+	}
+
+	// Test that the server will decline to resume at a higher version.
+	clientConfig.MaxVersion = VersionTLS11
+	state, _, err = testHandshake(clientConfig, serverConfig)
+	if err != nil {
+		t.Fatalf("handshake failed: %s", err)
+	}
+	if state.DidResume {
+		t.Fatalf("handshake resumed at a higher version")
+	}
+}
+
 // Note: see comment in handshake_test.go for details of how the reference
 // tests work.