-
Notifications
You must be signed in to change notification settings - Fork 2k
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
CHIA-2102 - Set minimum to TLSv1.3 #19079
Draft
emlowe
wants to merge
3
commits into
main
Choose a base branch
from
EL.tls1-3-minimum
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
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
emlowe
added
the
Changed
Required label for PR that categorizes merge commit message as "Changed" for changelog
label
Dec 20, 2024
emlowe
changed the title
[CHIA-2102]- Set minimum to TLSv1.3
CHIA-2102 - Set minimum to TLSv1.3
Dec 20, 2024
hoffmang9
previously approved these changes
Dec 20, 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.
Looks GREAT to me!
testssl.sh report:
|
hoffmang9
approved these changes
Dec 20, 2024
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Changed
Required label for PR that categorizes merge commit message as "Changed" for changelog
coverage-diff
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.
Set the minimum for all TLS communications to TLS v1.3. This includes the node peer protocol and service RPC calls.
Note, TLS v1.3 only has secure and safe ciphersuites, so we don't need to specifically set the ciphersuites list.
The change to allow TLS v1.2 was done here (#9195). At that time there was some concern over older python versions and older systems with openssl without 1.3 support.
Since that time we have dropped support for python 3.7 and 3.8 and it seems unlikely there are any system out there without TLS v1.3 support.
I'll note that apparently .NET still does not have TLS v1.3 support on macOS (dotnet/runtime#1979) until .NET 10 something in 2025 - so the daemon workaround introduced here (#16747) remains available to downgrade daemon connections to TLS v1.2
Converted to draft, because I believe this likely breaks again https://github.com/dkackman/chia-dotnet - that talks to the daemon and all the RPC endpoints.
Previously, the default was TLS 1.2 for everything except for the daemon which used 1.3. This broke
chia-dotnet
for the daemon connections only, but not for the RPC ports.Changing the default everywhere to 1.3 likely breaks
chia-dotnet
again for the RPC services and so those also need a downgrade. Unfortunately doing a downgrade for the RPC services and 1.3 on the peer protocol is suitably annoying.Discussed this with @dkackman and once dotnet 10 adds in support for TLS 1.3 (which is planned for early 2025) we can move forward with this.