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 50f4fea5bc942b6c5cc9fbe75c256df68073ae23..ffe9d26dc79ad3fbafca6d6a64bd74c12459382a 100644 (file)
--- a/doc/go1.15.html
+++ b/doc/go1.15.html
@@ -478,6 +478,12 @@ Do not send CLs removing the interior tags from such phrases.
       <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 fd21ae8fb10e3925b33b47ba8eb76e64e2b50116..3a5ca226133c493121e5a1be001caeac1dedbb8c 100644 (file)
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -278,6 +278,8 @@ type ClientSessionState struct {
        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 40c8e02c53dc7f980581f66b755e38dddf4fbaf5..46b0a770d5309436660038f77b3f6d08e32eaa60 100644 (file)
--- a/src/crypto/tls/handshake_client.go
+++ b/src/crypto/tls/handshake_client.go
@@ -728,10 +728,17 @@ func (hs *clientHandshakeState) processServerHello() (bool, error) {
                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 @@ func (hs *clientHandshakeState) readSessionTicket() error {
                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 1cda90190cec2e7e3533199bb4dcae85b8b84248..12b0254123e938a71dd6b8c0592db9cf235bf255 100644 (file)
--- a/src/crypto/tls/handshake_client_test.go
+++ b/src/crypto/tls/handshake_client_test.go
@@ -19,6 +19,7 @@ import (
        "os"
        "os/exec"
        "path/filepath"
+       "reflect"
        "strconv"
        "strings"
        "testing"
@@ -2430,3 +2431,83 @@ func TestDowngradeCanary(t *testing.T) {
                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 35a00f2f3a4c87376f9cdc8e2518541eb721f6f6..9c61105cf73d02b18b3c110c15777146d515677b 100644 (file)
--- a/src/crypto/tls/handshake_client_tls13.go
+++ b/src/crypto/tls/handshake_client_tls13.go
@@ -334,6 +334,8 @@ func (hs *clientHandshakeStateTLS13) processServerHello() error {
        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 @@ func (c *Conn) handleNewSessionTicket(msg *newSessionTicketMsgTLS13) error {
                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)