-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Upgrade to Go 1.20 #427
Upgrade to Go 1.20 #427
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! will likely merge this soon
Co-authored-by: Jaime Pillora <dev@jpillora.com>
jpillora, thanks for your review. |
hey @cmenginnz I bumped modules, and merged this PR into https://github.com/jpillora/chisel/compare/go120 but still failing (https://github.com/jpillora/chisel/actions/runs/5785559596) did you see this failure as well? |
Hey @jpillora, I pushed a commit into this PR to fix the test failure. Thanks. |
Oh, I'm hoping we can use the same fingerprints - the standard lib explicitly says that it's not deterministic, so we might need to change the key gen library - and offer an easier way generate key files |
based on this golang/go#38548 (comment) Go is guarding against chisel's use case explicitly - bad decision by me early on to have a we have two options:
option 1 could be considered insecure, so I think if we should support both 1 and 2, and if we go with 1 it should print the deprecation message above and suggest 2 |
@jpillora yes, it's good if we can use the same fingerprint. I will try to support options 1 and 2. Currently, in my PR, the argument name is "--private-key-file". Could you confirm that you want to rename it to "--keyfile"? Thanks. |
Yea I think keyfile matches keyseed? Is there a dash between I’m on mobile
atm
…On Tue, 8 Aug 2023 at 9:43 AM cmeng ***@***.***> wrote:
@jpillora <https://github.com/jpillora> yes, it's good if we can use the
same fingerprint. I will try to support options 1 and 2.
Currently, in my PR, the argument name is "--private-key-file". Could you
confirm that you want to rename it to "--keyfile"? Thanks.
—
Reply to this email directly, view it on GitHub
<#427 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAE2X47ESJRGSUDFBW7L5ELXUF4RTANCNFSM6AAAAAAYF5YS5I>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@jpillora, Following your comments, both options are implemented.
|
Closed with #440 |
Due to changes in the internal implementation of Go, upgrading from Go 1.19 (or earlier) to Go 1.20 will have two impacts on Chisel:
Generating different private keys implies generating different fingerprints.
My software heavily relies on Chisel. There are hundreds of client nodes that store their own fingerprints. When upgrading to Go 1.20, the changed fingerprints will cause all client nodes to lose connections to the server.
This PR attempts to address the two issues above by: