-
Notifications
You must be signed in to change notification settings - Fork 66
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.22 and revert support for new goboring #1863
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1863 +/- ##
===========================================
- Coverage 79.10% 38.62% -40.48%
===========================================
Files 173 335 +162
Lines 16201 43595 +27394
===========================================
+ Hits 12815 16840 +4025
- Misses 3066 26252 +23186
- Partials 320 503 +183 ☔ View full report in Codecov by Sentry. |
benjaminapetersen
approved these changes
Feb 8, 2024
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.
lgtm
- Current goboring only allows TLS 1.2. - The next goboring will allow TLS 1.2 and TLS 1.3. We got a preview of this when the Go team upgraded goboring in Go 1.21.6, but then downgraded it again in the next Go releases.
cfryanr
force-pushed
the
revert_support_for_new_goboring
branch
from
February 8, 2024 17:43
e9ee87a
to
c1fdb54
Compare
Goboring only allows TLS 1.2. The next goboring will allow both TLS 1.2 and TLS 1.3. We got a preview of this when the Go team upgraded goboring in Go 1.21.6, but then downgraded it again in the next Go releases. When the Go team eventually upgrades goboring again, then we can revert this commit to bring back TLS 1.3 support in FIPS mode.
cfryanr
force-pushed
the
revert_support_for_new_goboring
branch
from
February 8, 2024 18:44
c1fdb54
to
d279411
Compare
cfryanr
changed the title
Revert support for new goboring
Upgrade to Go 1.22 and revert support for new goboring
Feb 8, 2024
cfryanr
force-pushed
the
revert_support_for_new_goboring
branch
from
February 8, 2024 22:14
1e2d84a
to
904a60f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, Go released an upgrade to goboring in Go 1.21.6. They were apparently racing to meet a FIPS deadline for TLS 1.3 support, which is why they made such a big change in a patch release.
However, they later changed their minds and Go 1.21.7 and Go 1.22 are downgraded back to the same old goboring. See golang/go#65321.
We previously changed the Pinniped code to support TLS 1.3 because the new goboring supported it. See #1841. However, by upgrading to Go 1.22, now we are back to using the old goboring, so we need to revert our previous changes.
Note that the next release of Pinnped will not work if compiled for FIPS using Go 1.21.6 due to this. Any other recent version should work, because only Go 1.21.6 included the new goboring. This should not be a problem because the
hack/Dockerfile_fips
file will not reference Go 1.21.6 anymore in the next release of Pinniped. However, this is worth noting for anyone who is compiling Pinniped for FIPS using a custom Dockerfile or who is overriding theBUILD_IMAGE
arg of the Dockerfile.When Go someday upgrades goboring again, we should be able to revert the one related commit from this PR to enable TLS 1.3 in FIPS mode again (the commit titled "Revert support TLS 1.3 in FIPS mode because Go reverted goboring upgrade").
This PR also includes other changes needed for Go 1.22, as well as changes in the auto-generated code due to upgrading conroller-gen in CI to v0.14.0 (which was done at the same time as working on this PR).
Release note:
None because we never included FIPS mode TLS 1.3 in a Pinniped release, even though the code lived on the main branch for a little while.