From: Daniel McCarney Date: Wed, 26 Feb 2025 20:59:25 +0000 (-0500) Subject: crypto/tls: reject TLS 1.3 compat session ID in TLS 1.2 X-Git-Tag: go1.25rc1~777 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=574a9fa60e593154fbbe64f992b7e6656e3ab0b7;p=gostls13.git 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 Reviewed-by: Roland Shoemaker Reviewed-by: Junyang Shao Reviewed-by: Filippo Valsorda --- diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json index 6a9a6dfcc5..5261a35ca9 100644 --- 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 f6930c5d1b..1b6d672875 100644 --- 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