Skip to content
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.

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

crypto/tls: panic in TransportTLS handshake in Windows crashes app with panic [1.10 backport] #25033

Closed
gopherbot opened this issue Apr 24, 2018 · 7 comments

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 24, 2018

@FiloSottile requested issue #21376 to be considered for backport to the next 1.10 minor release.

@gopherbot please file this for backport.

See @aclements's comment #21376 (comment) about having to develop a different fix from CL 106275 in order not to break a public API.

@andybons

This comment has been minimized.

Copy link
Member

@andybons andybons commented Apr 27, 2018

Moving to 1.10.3 since this won't make it in 1.10.2 @aclements

@andybons andybons modified the milestones: Go1.10.2, Go1.10.3 Apr 27, 2018
@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Apr 29, 2018

@andybons according to @aclements (see above) you won't be able to cherry-pick CL 106275 here. Let me know if and when you want me to build custom solution for this issue.

Thank you.

Alex

@andybons

This comment has been minimized.

Copy link
Member

@andybons andybons commented Apr 30, 2018

@alexbrainman Since 1.10.2 is on the way out the door, we can aim for this to be in by 1.10.3 (May 1). Feel free to develop a patch on the release branch at your leisure. No rush since we have a month :)

@andybons

This comment has been minimized.

Copy link
Member

@andybons andybons commented Apr 30, 2018

Note this should also be included in 1.9.7 (#25034)

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented May 1, 2018

Feel free to develop a patch on the release branch at your leisure. No rush since we have a month :)

SGTM

Alex

@gopherbot

This comment has been minimized.

Copy link
Author

@gopherbot gopherbot commented May 5, 2018

Change https://golang.org/cl/111715 mentions this issue: [release-branch.go1.10] crypto/tls: copy and use adjusted syscall.CertChainPolicyPara

@gopherbot

This comment has been minimized.

Copy link
Author

@gopherbot gopherbot commented May 7, 2018

Closed by merging f858dbd to release-branch.go1.10.

@gopherbot gopherbot closed this May 7, 2018
gopherbot pushed a commit that referenced this issue May 7, 2018
…tChainPolicyPara

As discussed in issue #21376, it is unsafe to have
syscall.CertChainPolicyPara.ExtraPolicyPara uintptr -
it has to be a pointer type. So copy syscall.CertChainPolicyPara
into crypto/tls package, make ExtraPolicyPara unsafe.Pointer,
and use new struct instead of syscall.CertChainPolicyPara.

Fixes #25033

Change-Id: If914af056cbbb0c4d93ffaa915b3d2cb5ecad0cd
Reviewed-on: https://go-review.googlesource.com/111715
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
@golang golang locked and limited conversation to collaborators May 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.