From 12fa20f9e0eadb266e05461d2b6d174d915f1003 Mon Sep 17 00:00:00 2001 From: yuhan6665 <1588741+yuhan6665@users.noreply.github.com> Date: Sat, 10 May 2025 16:53:52 -0400 Subject: [PATCH] =?UTF-8?q?crypto/tls:=20add=20ConnectionState.CurveID=20T?= =?UTF-8?q?his=20required=20adding=20a=20new=20field=20to=20SessionState?= =?UTF-8?q?=20for=20TLS=201.0=E2=80=931.2,=20since=20the=20key=20exchange?= =?UTF-8?q?=20is=20not=20repeated=20on=20resumption.=20The=20additional=20?= =?UTF-8?q?field=20is=20unfortunately=20not=20backwards=20compatible=20bec?= =?UTF-8?q?ause=20current=20Go=20versions=20check=20that=20the=20encoding?= =?UTF-8?q?=20has=20no=20extra=20data=20at=20the=20end,=20but=20will=20cau?= =?UTF-8?q?se=20cross-version=20tickets=20to=20be=20ignored.=20Relaxed=20t?= =?UTF-8?q?hat=20so=20we=20can=20add=20fields=20in=20a=20backwards=20compa?= =?UTF-8?q?tible=20way=20the=20next=20time.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the cipher suite, we check that the session's is still acceptable per the Config. That would arguably make sense here, too: if a Config for example requires PQ, we should reject resumptions of connections that didn't use PQ. However, that only applies to pre-TLS 1.3 connections, since in TLS 1.3 we always do a fresh key exchange on resumption. Since PQ is the only main differentiator between key exchanges (aside from off-by-default non-PFS RSA, which are controlled by the cipher suite in TLS 1.0–1.2) and it's PQ-only, we can skip that check. Fixes #67516 Change-Id: I6a6a465681a6292edf66c7b8df8f4aba4171a76b Reviewed-on: https://go-review.googlesource.com/c/go/+/653315 Reviewed-by: David Chase Auto-Submit: Filippo Valsorda Reviewed-by: Daniel McCarney LUCI-TryBot-Result: Go LUCI Reviewed-by: Roland Shoemaker --- common.go | 9 ++++--- conn.go | 3 +-- handshake_client.go | 4 ++- handshake_server.go | 1 + ticket.go | 65 ++++++++++++++++++++++++++------------------- 5 files changed, 47 insertions(+), 35 deletions(-) diff --git a/common.go b/common.go index 930c27a..1ae7a37 100644 --- a/common.go +++ b/common.go @@ -247,6 +247,11 @@ type ConnectionState struct { // TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_AES_128_GCM_SHA256). CipherSuite uint16 + // CurveID is the key exchange mechanism used for the connection. The name + // refers to elliptic curves for legacy reasons, see [CurveID]. If a legacy + // RSA key exchange is used, this value is zero. + CurveID CurveID + // NegotiatedProtocol is the application protocol negotiated with ALPN. NegotiatedProtocol string @@ -304,10 +309,6 @@ type ConnectionState struct { // testingOnlyDidHRR is true if a HelloRetryRequest was sent/received. testingOnlyDidHRR bool - - // testingOnlyCurveID is the selected CurveID, or zero if an RSA exchanges - // is performed. - testingOnlyCurveID CurveID } // ExportKeyingMaterial returns length bytes of exported key material in a new diff --git a/conn.go b/conn.go index 6f792cd..14522a0 100644 --- a/conn.go +++ b/conn.go @@ -1694,8 +1694,7 @@ func (c *Conn) connectionStateLocked() ConnectionState { state.NegotiatedProtocol = c.clientProtocol state.DidResume = c.didResume state.testingOnlyDidHRR = c.didHRR - // c.curveID is not set on TLS 1.0–1.2 resumptions. Fix that before exposing it. - state.testingOnlyCurveID = c.curveID + state.CurveID = c.curveID state.NegotiatedProtocolIsMutual = true state.ServerName = c.serverName state.CipherSuite = c.cipherSuite diff --git a/handshake_client.go b/handshake_client.go index 9670b8f..522da0f 100644 --- a/handshake_client.go +++ b/handshake_client.go @@ -271,6 +271,7 @@ func (c *Conn) clientHandshake(ctx context.Context) (err error) { // This may be a renegotiation handshake, in which case some fields // need to be reset. c.didResume = false + c.curveID = 0 hello, keyShareKeys, ech, err := c.makeClientHello() if err != nil { @@ -951,10 +952,11 @@ func (hs *clientHandshakeState) processServerHello() (bool, error) { 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 + // connection, if any are provided. if len(c.scts) == 0 && len(hs.session.scts) != 0 { c.scts = hs.session.scts } + c.curveID = hs.session.curveID return true, nil } diff --git a/handshake_server.go b/handshake_server.go index 1216368..2c20a63 100644 --- a/handshake_server.go +++ b/handshake_server.go @@ -526,6 +526,7 @@ func (hs *serverHandshakeState) checkForResumption() error { c.extMasterSecret = sessionState.extMasterSecret hs.sessionState = sessionState hs.suite = suite + c.curveID = sessionState.curveID c.didResume = true return nil } diff --git a/ticket.go b/ticket.go index 37ac35b..6c2bff8 100644 --- a/ticket.go +++ b/ticket.go @@ -44,20 +44,21 @@ type SessionState struct { // case 0: Empty; // case 1: opaque alpn<1..2^8-1>; // }; - // select (SessionState.type) { - // case server: Empty; - // case client: struct { - // select (SessionState.version) { - // case VersionTLS10..VersionTLS12: Empty; - // case VersionTLS13: struct { - // uint64 use_by; - // uint32 age_add; - // }; + // select (SessionState.version) { + // case VersionTLS10..VersionTLS12: uint16 curve_id; + // case VersionTLS13: select (SessionState.type) { + // case server: Empty; + // case client: struct { + // uint64 use_by; + // uint32 age_add; // }; // }; // }; // } SessionState; // + // The format can be extended backwards-compatibly by adding new fields at + // the end. Otherwise, a new SessionStateType must be used, as different Go + // versions may share the same session ticket encryption key. // Extra is ignored by crypto/tls, but is encoded by [SessionState.Bytes] // and parsed by [ParseSessionState]. @@ -97,6 +98,9 @@ type SessionState struct { useBy uint64 // seconds since UNIX epoch ageAdd uint32 ticket []byte + + // TLS 1.0–1.2 only fields. + curveID CurveID } // Bytes encodes the session, including any private fields, so that it can be @@ -161,11 +165,13 @@ func (s *SessionState) Bytes() ([]byte, error) { b.AddBytes([]byte(s.alpnProtocol)) }) } - if s.isClient { - if s.version >= VersionTLS13 { + if s.version >= VersionTLS13 { + if s.isClient { addUint64(&b, s.useBy) b.AddUint32(s.ageAdd) } + } else { + b.AddUint16(uint16(s.curveID)) } return b.Bytes() } @@ -187,7 +193,6 @@ func ParseSessionState(data []byte) (*SessionState, error) { var extra cryptobyte.String if !s.ReadUint16(&ss.version) || !s.ReadUint8(&typ) || - (typ != 1 && typ != 2) || !s.ReadUint16(&ss.cipherSuite) || !readUint64(&s, &ss.createdAt) || !readUint8LengthPrefixed(&s, &ss.secret) || @@ -205,6 +210,14 @@ func ParseSessionState(data []byte) (*SessionState, error) { } ss.Extra = append(ss.Extra, e) } + switch typ { + case 1: + ss.isClient = false + case 2: + ss.isClient = true + default: + return nil, errors.New("tls: unknown session encoding") + } switch extMasterSecret { case 0: ss.extMasterSecret = false @@ -229,6 +242,9 @@ func ParseSessionState(data []byte) (*SessionState, error) { ss.activeCertHandles = append(ss.activeCertHandles, c) ss.peerCertificates = append(ss.peerCertificates, c.cert) } + if ss.isClient && len(ss.peerCertificates) == 0 { + return nil, errors.New("tls: no server certificates in client session") + } ss.ocspResponse = cert.OCSPStaple ss.scts = cert.SignedCertificateTimestamps var chainList cryptobyte.String @@ -266,24 +282,16 @@ func ParseSessionState(data []byte) (*SessionState, error) { } ss.alpnProtocol = string(alpn) } - if isClient := typ == 2; !isClient { - if !s.Empty() { + if ss.version >= VersionTLS13 { + if ss.isClient { + if !s.ReadUint64(&ss.useBy) || !s.ReadUint32(&ss.ageAdd) { + return nil, errors.New("tls: invalid session encoding") + } + } + } else { + if !s.ReadUint16((*uint16)(&ss.curveID)) { return nil, errors.New("tls: invalid session encoding") } - return ss, nil - } - ss.isClient = true - if len(ss.peerCertificates) == 0 { - return nil, errors.New("tls: no server certificates in client session") - } - if ss.version < VersionTLS13 { - if !s.Empty() { - return nil, errors.New("tls: invalid session encoding") - } - return ss, nil - } - if !s.ReadUint64(&ss.useBy) || !s.ReadUint32(&ss.ageAdd) || !s.Empty() { - return nil, errors.New("tls: invalid session encoding") } return ss, nil } @@ -303,6 +311,7 @@ func (c *Conn) sessionState() *SessionState { isClient: c.isClient, extMasterSecret: c.extMasterSecret, verifiedChains: c.verifiedChains, + curveID: c.curveID, } }