From e10e39db4d2901d236d5e16f8daf4dca966f2d20 Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Tue, 28 Jul 2020 11:27:46 +0800 Subject: [PATCH] crypto/tls: make checkForResumption side-effect free Fixes #39406 When use checkForResumption() function it could be side-effect sometime. 1. hs.sessionState will be changed and retained although the function return false. 2. hs.suite will be changed and retained when the statements below return false. So we should use a local variable, cilentSessionState, to replace hs.sessionState in the function. And move the set-suite statements down to avoid being changed too early. --- src/crypto/tls/handshake_server.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go index 16d3e643f0b28e..a86dc6d4a55a5e 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -357,26 +357,26 @@ func (hs *serverHandshakeState) checkForResumption() bool { if plaintext == nil { return false } - hs.sessionState = &sessionState{usedOldKey: usedOldKey} - ok := hs.sessionState.unmarshal(plaintext) + clientSessionState := &sessionState{usedOldKey: usedOldKey} + ok := clientSessionState.unmarshal(plaintext) if !ok { return false } - createdAt := time.Unix(int64(hs.sessionState.createdAt), 0) + createdAt := time.Unix(int64(clientSessionState.createdAt), 0) if c.config.time().Sub(createdAt) > maxSessionTicketLifetime { return false } // Never resume a session for a different TLS version. - if c.vers != hs.sessionState.vers { + if c.vers != clientSessionState.vers { return false } cipherSuiteOk := false // Check that the client is still offering the ciphersuite in the session. for _, id := range hs.clientHello.cipherSuites { - if id == hs.sessionState.cipherSuite { + if id == clientSessionState.cipherSuite { cipherSuiteOk = true break } @@ -385,14 +385,7 @@ func (hs *serverHandshakeState) checkForResumption() bool { return false } - // Check that we also support the ciphersuite from the session. - hs.suite = selectCipherSuite([]uint16{hs.sessionState.cipherSuite}, - c.config.cipherSuites(), hs.cipherSuiteOk) - if hs.suite == nil { - return false - } - - sessionHasClientCerts := len(hs.sessionState.certificates) != 0 + sessionHasClientCerts := len(clientSessionState.certificates) != 0 needClientCerts := requiresClientCert(c.config.ClientAuth) if needClientCerts && !sessionHasClientCerts { return false @@ -401,6 +394,15 @@ func (hs *serverHandshakeState) checkForResumption() bool { return false } + // Check that we also support the ciphersuite from the session. + hs.suite = selectCipherSuite([]uint16{clientSessionState.cipherSuite}, + c.config.cipherSuites(), hs.cipherSuiteOk) + if hs.suite == nil { + return false + } + + hs.sessionState = clientSessionState + return true }