crypto/tls: only set LocalCertificate when a certificate is presented#79968
crypto/tls: only set LocalCertificate when a certificate is presented#79968sin99xx wants to merge 1 commit into
Conversation
ConnectionState.LocalCertificate is documented to be populated only for connections which are not resumed. However, the TLS 1.0-1.2 server handshake set it in processClientHello, which runs before checkForResumption, and nothing cleared it on the abbreviated handshake path. Resumed pre-1.3 server connections therefore reported a certificate chain that was never presented to the peer (also visible to VerifyConnection and WrapSession during the handshake). Set the field in doFullHandshake, where the Certificate message is actually written, so it is populated if and only if a chain was presented. TLS 1.3 is unaffected because pickCertificate already returns early for PSK resumption. Also pin MaxVersion in TestLocalCertificate and TestLocalCertificateResumption: they previously set only MinVersion, so every pre-1.3 subtest silently negotiated TLS 1.3 and the broken path had no coverage. With MaxVersion pinned, the resumption test fails without this fix at TLS 1.0-1.2. Fixes golang#79967
|
This PR (HEAD: e1a072c) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/789862. Important tips:
|
|
Message from Gopher Robot: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/789862. |
|
Message from Gopher Robot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/789862. |
|
Message from Daniel McCarney: Patch Set 1: Commit-Queue+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/789862. |
|
Message from golang-scoped@luci-project-accounts.iam.gserviceaccount.com: Patch Set 1: Dry run: CV is trying the patch. Bot data: {"action":"start","triggered_at":"2026-06-12T12:18:43Z","revision":"7560d6df9f14f449ee3d5bb1caa85e149c7921dc"} Please don’t reply on this GitHub thread. Visit golang.org/cl/789862. |
|
Message from Daniel McCarney: Patch Set 1: -Commit-Queue (Performed by <GERRIT_ACCOUNT_60063> on behalf of <GERRIT_ACCOUNT_26879>) Please don’t reply on this GitHub thread. Visit golang.org/cl/789862. |
|
Message from golang-scoped@luci-project-accounts.iam.gserviceaccount.com: Patch Set 1: This CL has failed the run. Reason: Tryjob golang/try/gotip-linux-arm64_debian13 has failed with summary (view all results):
To reproduce, try Additional links for debugging: Please don’t reply on this GitHub thread. Visit golang.org/cl/789862. |
|
Message from golang-scoped@luci-project-accounts.iam.gserviceaccount.com: Patch Set 1: LUCI-TryBot-Result-1 Please don’t reply on this GitHub thread. Visit golang.org/cl/789862. |
|
Message from Roland Shoemaker: Patch Set 1: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/789862. |
ConnectionState.LocalCertificate is documented to be populated only for connections which are not resumed. However, the TLS 1.0-1.2 server handshake set it in processClientHello, which runs before checkForResumption, and nothing cleared it on the abbreviated handshake path. Resumed pre-1.3 server connections therefore reported a certificate chain that was never presented to the peer (also visible to VerifyConnection and WrapSession during the handshake).
Set the field in doFullHandshake, where the Certificate message is actually written, so it is populated if and only if a chain was presented. TLS 1.3 is unaffected because pickCertificate already returns early for PSK resumption.
Also pin MaxVersion in TestLocalCertificate and TestLocalCertificateResumption: they previously set only MinVersion, so every pre-1.3 subtest silently negotiated TLS 1.3 and the broken path had no coverage. With MaxVersion pinned, the resumption test fails without this fix at TLS 1.0-1.2.
Verified: strengthened resumption test fails on unpatched master at TLS 1.0/1.1/1.2 and passes with the fix; full
go test crypto/tlspasses; standalone repro from the issue prints len(LocalCertificate)=0 for resumed TLS 1.2 connections.Fixes #79967