crypto/tls: restore OCSP and SCTs during session resumption
Restore previously sent SCTs and stapled OCSP response during session
resumption for both TLS 1.2 and 1.3. This behavior is somewhat
complicated for TLS 1.2 as SCTs are sent during the server hello,
so they override what is saved in ClientSessionState. It is likely
that if the server is sending a different set of SCTs there is probably
a reason for doing so, such as a log being retired, or SCT validation
requirements changing, so it makes sense to defer to the server in
that case.
Fixes #39075
Change-Id: I3c0fa2f69c6bf0247a447c48a1b4c733a882a233
Reviewed-on: https://go-review.googlesource.com/c/go/+/234237
Reviewed-by: Filippo Valsorda <filippo@golang.org>
diff --git a/doc/go1.15.html b/doc/go1.15.html
index 50f4fea..ffe9d26 100644
--- a/doc/go1.15.html
+++ b/doc/go1.15.html
@@ -478,6 +478,12 @@
<a href="/pkg/crypto/tls/#ClientAuthType"><code>ClientAuthType</code></a>
now implement <a href="/pkg/fmt/#Stringer"><code>fmt.Stringer</code></a>.
</p>
+
+ <p><!-- CL 236737 -->
+ The <a href="/pkg/crypto/tls/#ConnectionState"><code>ConnectionState</code></a>
+ fields <code>OCSPResponse</code> and <code>SignedCertificateTimestamps</code>
+ are now repopulated on client-side resumed connections.
+ </p>
</dd>
</dl><!-- crypto/tls -->
diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index fd21ae8..3a5ca22 100644
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -278,6 +278,8 @@
serverCertificates []*x509.Certificate // Certificate chain presented by the server
verifiedChains [][]*x509.Certificate // Certificate chains we built for verification
receivedAt time.Time // When the session ticket was received from the server
+ ocspResponse []byte // Stapled OCSP response presented by the server
+ scts [][]byte // SCTs presented by the server
// TLS 1.3 fields.
nonce []byte // Ticket nonce sent by the server, to derive PSK
diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go
index 40c8e02..46b0a77 100644
--- a/src/crypto/tls/handshake_client.go
+++ b/src/crypto/tls/handshake_client.go
@@ -728,10 +728,17 @@
return false, errors.New("tls: server resumed a session with a different cipher suite")
}
- // Restore masterSecret and peerCerts from previous state
+ // Restore masterSecret, peerCerts, and ocspResponse from previous state
hs.masterSecret = hs.session.masterSecret
c.peerCertificates = hs.session.serverCertificates
c.verifiedChains = hs.session.verifiedChains
+ c.ocspResponse = hs.session.ocspResponse
+ // Let the ServerHello SCTs override the session SCTs from the original
+ // connection, if any are provided
+ if len(c.scts) == 0 && len(hs.session.scts) != 0 {
+ c.scts = hs.session.scts
+ }
+
return true, nil
}
@@ -788,6 +795,8 @@
serverCertificates: c.peerCertificates,
verifiedChains: c.verifiedChains,
receivedAt: c.config.time(),
+ ocspResponse: c.ocspResponse,
+ scts: c.scts,
}
return nil
diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go
index 1cda901..12b0254 100644
--- a/src/crypto/tls/handshake_client_test.go
+++ b/src/crypto/tls/handshake_client_test.go
@@ -19,6 +19,7 @@
"os"
"os/exec"
"path/filepath"
+ "reflect"
"strconv"
"strings"
"testing"
@@ -2430,3 +2431,83 @@
t.Errorf("client unexpectedly reacted to a canary in TLS 1.0")
}
}
+
+func TestResumptionKeepsOCSPAndSCT(t *testing.T) {
+ t.Run("TLSv12", func(t *testing.T) { testResumptionKeepsOCSPAndSCT(t, VersionTLS12) })
+ t.Run("TLSv13", func(t *testing.T) { testResumptionKeepsOCSPAndSCT(t, VersionTLS13) })
+}
+
+func testResumptionKeepsOCSPAndSCT(t *testing.T, ver uint16) {
+ issuer, err := x509.ParseCertificate(testRSACertificateIssuer)
+ if err != nil {
+ t.Fatalf("failed to parse test issuer")
+ }
+ roots := x509.NewCertPool()
+ roots.AddCert(issuer)
+ clientConfig := &Config{
+ MaxVersion: ver,
+ ClientSessionCache: NewLRUClientSessionCache(32),
+ ServerName: "example.golang",
+ RootCAs: roots,
+ }
+ serverConfig := testConfig.Clone()
+ serverConfig.MaxVersion = ver
+ serverConfig.Certificates[0].OCSPStaple = []byte{1, 2, 3}
+ serverConfig.Certificates[0].SignedCertificateTimestamps = [][]byte{{4, 5, 6}}
+
+ _, ccs, err := testHandshake(t, clientConfig, serverConfig)
+ if err != nil {
+ t.Fatalf("handshake failed: %s", err)
+ }
+ // after a new session we expect to see OCSPResponse and
+ // SignedCertificateTimestamps populated as usual
+ if !bytes.Equal(ccs.OCSPResponse, serverConfig.Certificates[0].OCSPStaple) {
+ t.Errorf("client ConnectionState contained unexpected OCSPResponse: wanted %v, got %v",
+ serverConfig.Certificates[0].OCSPStaple, ccs.OCSPResponse)
+ }
+ if !reflect.DeepEqual(ccs.SignedCertificateTimestamps, serverConfig.Certificates[0].SignedCertificateTimestamps) {
+ t.Errorf("client ConnectionState contained unexpected SignedCertificateTimestamps: wanted %v, got %v",
+ serverConfig.Certificates[0].SignedCertificateTimestamps, ccs.SignedCertificateTimestamps)
+ }
+
+ // if the server doesn't send any SCTs, repopulate the old SCTs
+ oldSCTs := serverConfig.Certificates[0].SignedCertificateTimestamps
+ serverConfig.Certificates[0].SignedCertificateTimestamps = nil
+ _, ccs, err = testHandshake(t, clientConfig, serverConfig)
+ if err != nil {
+ t.Fatalf("handshake failed: %s", err)
+ }
+ if !ccs.DidResume {
+ t.Fatalf("expected session to be resumed")
+ }
+ // after a resumed session we also expect to see OCSPResponse
+ // and SignedCertificateTimestamps populated
+ if !bytes.Equal(ccs.OCSPResponse, serverConfig.Certificates[0].OCSPStaple) {
+ t.Errorf("client ConnectionState contained unexpected OCSPResponse after resumption: wanted %v, got %v",
+ serverConfig.Certificates[0].OCSPStaple, ccs.OCSPResponse)
+ }
+ if !reflect.DeepEqual(ccs.SignedCertificateTimestamps, oldSCTs) {
+ t.Errorf("client ConnectionState contained unexpected SignedCertificateTimestamps after resumption: wanted %v, got %v",
+ oldSCTs, ccs.SignedCertificateTimestamps)
+ }
+
+ // Only test overriding the SCTs for TLS 1.2, since in 1.3
+ // the server won't send the message containing them
+ if ver == VersionTLS13 {
+ return
+ }
+
+ // if the server changes the SCTs it sends, they should override the saved SCTs
+ serverConfig.Certificates[0].SignedCertificateTimestamps = [][]byte{{7, 8, 9}}
+ _, ccs, err = testHandshake(t, clientConfig, serverConfig)
+ if err != nil {
+ t.Fatalf("handshake failed: %s", err)
+ }
+ if !ccs.DidResume {
+ t.Fatalf("expected session to be resumed")
+ }
+ if !reflect.DeepEqual(ccs.SignedCertificateTimestamps, serverConfig.Certificates[0].SignedCertificateTimestamps) {
+ t.Errorf("client ConnectionState contained unexpected SignedCertificateTimestamps after resumption: wanted %v, got %v",
+ serverConfig.Certificates[0].SignedCertificateTimestamps, ccs.SignedCertificateTimestamps)
+ }
+}
diff --git a/src/crypto/tls/handshake_client_tls13.go b/src/crypto/tls/handshake_client_tls13.go
index 35a00f2..9c61105 100644
--- a/src/crypto/tls/handshake_client_tls13.go
+++ b/src/crypto/tls/handshake_client_tls13.go
@@ -334,6 +334,8 @@
c.didResume = true
c.peerCertificates = hs.session.serverCertificates
c.verifiedChains = hs.session.verifiedChains
+ c.ocspResponse = hs.session.ocspResponse
+ c.scts = hs.session.scts
return nil
}
@@ -666,6 +668,8 @@
nonce: msg.nonce,
useBy: c.config.time().Add(lifetime),
ageAdd: msg.ageAdd,
+ ocspResponse: c.ocspResponse,
+ scts: c.scts,
}
cacheKey := clientSessionCacheKey(c.conn.RemoteAddr(), c.config)
