crypto/tls: reject TLS 1.3 compat session ID in TLS 1.2

If we weren't resuming an existing session, and we constructed a TLS 1.3
compatible client hello, ensure the server doesn't echo back the
made up compatibility session ID if we end up handshaking for TLS 1.2.

As part of an effort to make the initial stages of a TLS 1.3 handshake
compatible with TLS 1.2 middleboxes, TLS 1.3 requires that the client
hello contain a non-empty legacy_session_id value. For anti-ossification
purposes it's recommended this ID be randomly generated. This is the
strategy the crypto/tls package takes.

When we follow this approach, but then end up negotiating TLS 1.2, the
server should not have echoed back that random ID to us. It's impossible
for the server to have had a session with a matching ID and so it is
misbehaving and it's prudent for our side to abort the handshake.

See RFC 8446 Section 4.1.2 for more detail:
  https://www.rfc-editor.org/rfc/rfc8446#section-4.1.2

Adopting this behaviour allows un-ignoring the BoGo
EchoTLS13CompatibilitySessionID testcase.

Updates #72006

Change-Id: I1e52075177a13a7aa103b45498eae38d8a4c34b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/652997
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>

diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json
index 6a9a6dfcc5f8b0f68b9d63d60247452ba4989679..5261a35ca95bad2a7ff38805334f86a34ae34cfb 100644 (file)
--- a/src/crypto/tls/bogo_config.json
+++ b/src/crypto/tls/bogo_config.json
@@ -55,7 +55,6 @@
         "KyberKeyShareIncludedThird": "we always send the Kyber key share first",
         "GREASE-Server-TLS13": "We don't send GREASE extensions",
         "SendBogusAlertType": "sending wrong alert type",
-        "EchoTLS13CompatibilitySessionID": "TODO reject compat session ID",
         "*Client-P-224*": "no P-224 support",
         "*Server-P-224*": "no P-224 support",
         "CurveID-Resume*": "unexposed curveID is not stored in the ticket yet",

diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go
index f6930c5d1b7381b65f1d67b5805db706f8e14e09..1b6d67287538e73e9ea646836962e871c15b7909 100644 (file)
--- a/src/crypto/tls/handshake_client.go
+++ b/src/crypto/tls/handshake_client.go
@@ -557,6 +557,19 @@ func (c *Conn) pickTLSVersion(serverHello *serverHelloMsg) error {
 func (hs *clientHandshakeState) handshake() error {
        c := hs.c
 
+       // If we did not load a session (hs.session == nil), but we did set a
+       // session ID in the transmitted client hello (hs.hello.sessionId != nil),
+       // it means we tried to negotiate TLS 1.3 and sent a random session ID as a
+       // compatibility measure (see RFC 8446, Section 4.1.2).
+       //
+       // Since we're now handshaking for TLS 1.2, if the server echoed the
+       // transmitted ID back to us, we know mischief is afoot: the session ID
+       // was random and can't possibly be recognized by the server.
+       if hs.session == nil && hs.hello.sessionId != nil && bytes.Equal(hs.hello.sessionId, hs.serverHello.sessionId) {
+               c.sendAlert(alertIllegalParameter)
+               return errors.New("tls: server echoed TLS 1.3 compatibility session ID in TLS 1.2")
+       }
+
        isResume, err := hs.processServerHello()
        if err != nil {
                return err