-
Notifications
You must be signed in to change notification settings - Fork 592
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
Icessl protocols #2026
Icessl protocols #2026
Conversation
One downside with this change is that the user can no longer change the protocols for the Ice services. Is this an issue? |
We can keep it and support only 1.2 and 1.3, there is no reason to support anything older. Would you prefer that? I don't see a reason to allow enabling older versions, which might be disabled by the platform and not work. |
Good point, I forgot this setting is not useful with newer versions of TLS. |
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 good. Should we explicitly enable only TLS 1.2 and 1.3 or just rely on what the defaults are set to?
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.
I agree: this is an advanced feature, and if you want it, you should use the programmatic API we offer in 3.8.
This PR removes support for setting SSL protocols using IceSSL properties,
IceSSL.Protocols
,IceSSL.ProtocolVersionMax
, andIceSSL.ProtocolVersionMin
.The SSL transport would use the system defaults, there is usually no need to change this. For advanced use cases, you can change the system defaults or use the IceSSL native API.