Skip to content

crypto/tls: segfault when calling tlsrsakex.IncNonDefault() [1.22 backport] #65994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
gopherbot opened this issue Feb 28, 2024 · 7 comments
Closed

crypto/tls: segfault when calling tlsrsakex.IncNonDefault() [1.22 backport] #65994

gopherbot opened this issue Feb 28, 2024 · 7 comments
Labels
CherryPickApproved Used during the release process for point releases
Milestone
Go1.22.5

Comments

@gopherbot
Copy link
Contributor

@FiloSottile requested issue #65991 to be considered for backport to the next 1.22 minor release.

@gopherbot please open a Go 1.22 backport issue. This is a severe regression in Go+BoringCrypto mode.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 28, 2024
@gopherbot gopherbot mentioned this issue Feb 28, 2024
Closed
@gopherbot gopherbot modified the milestones: Go1.22.1, Go1.22.2 Feb 28, 2024
@gopherbot gopherbot modified the milestones: Go1.22.2, Go1.22.3 Apr 3, 2024
@gopherbot gopherbot modified the milestones: Go1.22.3, Go1.22.4 May 7, 2024
@cagedmantis
Copy link
Contributor

Approved as this is a bug without a workaround.

@cagedmantis cagedmantis added the CherryPickApproved Used during the release process for point releases label May 22, 2024
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label May 22, 2024
@mdempsky
Copy link
Contributor

mdempsky commented May 29, 2024 •
edited by gabyhelp
Loading

@FiloSottile It looks like this requires both CL 582315 and CL 586755 (no tests) to be backported? I can create the CL, but I'm unfamiliar with Go's BoringSSL-specific details.

@gopherbot gopherbot modified the milestones: Go1.22.4, Go1.22.5 Jun 4, 2024
@michaelbeaumont
Copy link
Contributor

michaelbeaumont commented Jun 5, 2024 •
edited
Loading

Why was this skipped for this release? Was there discussion around this?
This makes goboring unusable with 1.22. 😞 @FiloSottile @mdempsky

EDIT: ok, it's my understanding that 1.22.4 was a security release, so this was pushed. Does the release of 1.22.4 push the release of this fix even further?

@slonka slonka mentioned this issue Jun 12, 2024
Closed
@dmitshur
Copy link
Member

This backport is approved and it's waiting on a cherry-pick CL to be created. See https://go.dev/wiki/MinorReleases#making-cherry-pick-cls. Once that's created and all of its submit requirements are satisfied, it can be submitted and included in the nearest upcoming minor release.

@michaelbeaumont
Copy link
Contributor

@dmitshur created: https://go-review.googlesource.com/c/go/+/593395

@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/593395 mentions this issue: [release-branch.go1.22] crypto/tls: don't call tlsrsakex.IncNonDefault with FIPS

gopherbot pushed a commit that referenced this issue Jun 24, 2024
@michaelbeaumont @joedian
[release-branch.go1.22] crypto/tls: don't call tlsrsakex.IncNonDefaul…
81fc616 
…t with FIPS

We haven't called tlsrsakex.Value() yet at this point if we're using
FIPS, like if CipherSuites != nil. This adds needFIPS as a gate next to
CipherSuites != nil. FIPS specifies suites that would be skipped if
tlsarsakex were set.

For #65991.
Fixes #65994.

Change-Id: I8070d8f43f27c04067490af8cc7ec5e787f2b9bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/582315
Reviewed-by: Filippo Valsorda <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
TryBot-Bypass: Filippo Valsorda <[email protected]>
Auto-Submit: Filippo Valsorda <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
(cherry picked from commit 78e50d0)
Reviewed-on: https://go-review.googlesource.com/c/go/+/593395
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
@gopherbot
Copy link
Contributor Author

Closed by merging 81fc616 to release-branch.go1.22.

@ueno ueno mentioned this issue Jul 4, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases
Projects
None yet
Milestone
Go1.22.5
Development

No branches or pull requests
5 participants
@mdempsky @cagedmantis @dmitshur @michaelbeaumont @gopherbot