]> Cypherpunks repositories - gostls13.git/commitdiff
crypto/tls: Never resume sessions across different versions.
authorDavid Benjamin <davidben@google.com>
Mon, 15 Feb 2016 16:41:40 +0000 (11:41 -0500)
committerBrad Fitzpatrick <bradfitz@golang.org>
Wed, 18 May 2016 21:20:33 +0000 (21:20 +0000)
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>

src/crypto/tls/handshake_server.go
src/crypto/tls/handshake_server_test.go

index 8e94f2143acbc4763926f2658d25241bdca10306..cf617df19f5c0023d173fc6120718a76898de151 100644 (file)
@@ -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
        }
 
index fba81f619a6fe7f31cb10e503bb21b3a63b6d6b2..d878f9988949d783014e799a0dacb85a1b69d797 100644 (file)
@@ -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.