mirror of
https://github.com/golang/go
synced 2024-11-23 11:20:05 -07:00
crypto/tls: fix ConnectionState().VerifiedChains for resumed connection
Strengthening VerifyHostname exposed the fact that for resumed connections, ConnectionState().VerifiedChains was not being saved and restored during the ClientSessionCache operations. Do that. This change just saves the verified chains in the client's session cache. It does not re-verify the certificates when resuming a connection. There are arguments both ways about this: we want fast, light-weight resumption connections (thus suggesting that we shouldn't verify) but it could also be a little surprising that, if the verification config is changed, that would be ignored if the same session cache is used. On the server side we do re-verify client-auth certificates, but the situation is a little different there. The client session cache is an object in memory that's reset each time the process restarts. But the server's session cache is a conceptual object, held by the clients, so can persist across server restarts. Thus the chance of a change in verification config being surprisingly ignored is much higher in the server case. Fixes #12024. Change-Id: I3081029623322ce3d9f4f3819659fdd9a381db16 Reviewed-on: https://go-review.googlesource.com/13164 Reviewed-by: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Adam Langley <agl@golang.org>
This commit is contained in:
parent
0290d51b4a
commit
46a2913882
@ -191,11 +191,12 @@ const (
|
||||
// ClientSessionState contains the state needed by clients to resume TLS
|
||||
// sessions.
|
||||
type ClientSessionState struct {
|
||||
sessionTicket []uint8 // Encrypted ticket used for session resumption with server
|
||||
vers uint16 // SSL/TLS version negotiated for the session
|
||||
cipherSuite uint16 // Ciphersuite negotiated for the session
|
||||
masterSecret []byte // MasterSecret generated by client on a full handshake
|
||||
serverCertificates []*x509.Certificate // Certificate chain presented by the server
|
||||
sessionTicket []uint8 // Encrypted ticket used for session resumption with server
|
||||
vers uint16 // SSL/TLS version negotiated for the session
|
||||
cipherSuite uint16 // Ciphersuite negotiated for the session
|
||||
masterSecret []byte // MasterSecret generated by client on a full handshake
|
||||
serverCertificates []*x509.Certificate // Certificate chain presented by the server
|
||||
verifiedChains [][]*x509.Certificate // Certificate chains we built for verification
|
||||
}
|
||||
|
||||
// ClientSessionCache is a cache of ClientSessionState objects that can be used
|
||||
|
@ -547,6 +547,7 @@ func (hs *clientHandshakeState) processServerHello() (bool, error) {
|
||||
// Restore masterSecret and peerCerts from previous state
|
||||
hs.masterSecret = hs.session.masterSecret
|
||||
c.peerCertificates = hs.session.serverCertificates
|
||||
c.verifiedChains = hs.session.verifiedChains
|
||||
return true, nil
|
||||
}
|
||||
return false, nil
|
||||
@ -604,6 +605,7 @@ func (hs *clientHandshakeState) readSessionTicket() error {
|
||||
cipherSuite: hs.suite.id,
|
||||
masterSecret: hs.masterSecret,
|
||||
serverCertificates: c.peerCertificates,
|
||||
verifiedChains: c.verifiedChains,
|
||||
}
|
||||
|
||||
return nil
|
||||
|
@ -406,10 +406,20 @@ func TestClientResumption(t *testing.T) {
|
||||
CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA, TLS_ECDHE_RSA_WITH_RC4_128_SHA},
|
||||
Certificates: testConfig.Certificates,
|
||||
}
|
||||
|
||||
issuer, err := x509.ParseCertificate(testRSACertificateIssuer)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
|
||||
rootCAs := x509.NewCertPool()
|
||||
rootCAs.AddCert(issuer)
|
||||
|
||||
clientConfig := &Config{
|
||||
CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
|
||||
InsecureSkipVerify: true,
|
||||
ClientSessionCache: NewLRUClientSessionCache(32),
|
||||
RootCAs: rootCAs,
|
||||
ServerName: "example.golang",
|
||||
}
|
||||
|
||||
testResumeState := func(test string, didResume bool) {
|
||||
@ -420,6 +430,9 @@ func TestClientResumption(t *testing.T) {
|
||||
if hs.DidResume != didResume {
|
||||
t.Fatalf("%s resumed: %v, expected: %v", test, hs.DidResume, didResume)
|
||||
}
|
||||
if didResume && (hs.PeerCertificates == nil || hs.VerifiedChains == nil) {
|
||||
t.Fatalf("expected non-nil certificates after resumption. Got peerCertificates: %#v, verifedCertificates: %#v", hs.PeerCertificates, hs.VerifiedChains)
|
||||
}
|
||||
}
|
||||
|
||||
getTicket := func() []byte {
|
||||
|
@ -307,3 +307,28 @@ func TestVerifyHostname(t *testing.T) {
|
||||
t.Fatalf("verify www.google.com succeeded with InsecureSkipVerify=true")
|
||||
}
|
||||
}
|
||||
|
||||
func TestVerifyHostnameResumed(t *testing.T) {
|
||||
testenv.MustHaveExternalNetwork(t)
|
||||
|
||||
config := &Config{
|
||||
ClientSessionCache: NewLRUClientSessionCache(32),
|
||||
}
|
||||
for i := 0; i < 2; i++ {
|
||||
c, err := Dial("tcp", "www.google.com:https", config)
|
||||
if err != nil {
|
||||
t.Fatalf("Dial #%d: %v", i, err)
|
||||
}
|
||||
cs := c.ConnectionState()
|
||||
if i > 0 && !cs.DidResume {
|
||||
t.Fatalf("Subsequent connection unexpectedly didn't resume")
|
||||
}
|
||||
if cs.VerifiedChains == nil {
|
||||
t.Fatalf("Dial #%d: cs.VerifiedChains == nil", i)
|
||||
}
|
||||
if err := c.VerifyHostname("www.google.com"); err != nil {
|
||||
t.Fatalf("verify www.google.com #%d: %v", i, err)
|
||||
}
|
||||
c.Close()
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user