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

fix(net): switch to Tlsv1_2 for default ssl method #482

Closed
wants to merge 1 commit into from

Conversation

softprops
Copy link
Contributor

This change substitutes the previous insecure default ssl method with a more secure default. Without this I was blocked writing a client interface for a secure server supporting the more secure tls method and there wasn't a way I could find to provide the ssl method used but the client.

A more thorough changeset would probably involve something like adding a new ssl provider interface to then network connector and network listener interfaces that allow's for user defined ssl contexts, but I think that would be a breaking change. This was a much smaller change, makes the defaults more secure, and shouldn't be a breaking change, so I went with this instead.

@seanmonstar
Copy link
Member

This was actually using Sslv23 by default, which is Ssl v2, v3, and Tls v1. It seemed like by default it would make the most amount of the web work. If I'm wrong, I'd love to know!

However, I recently merged in #479, which allows you to create a SslContext yourself. Would that serve your purposes?

(side note: I'd consider a change like this to be feat(server), and probably a breaking change, since servers upgrading would silently be dropping Ssl v2 and v3 support.)

@softprops
Copy link
Contributor Author

I guess the server communicating with rejected anything less than [https://github.com/sfackler/rust-openssl/blob/master/openssl/src/ssl/mod.rs#L99]. I say the other pull but it looked like it only addressed the server side of the issue. The client side connectors still have the ssl method used hard coded :/

@seanmonstar
Copy link
Member

Oh, I see. This also affects HttpConnector (I misread and though this was server only).

If we make this change, does this mean the Client no longer works (by default) against servers that only use SSL instead of TLS?

@softprops
Copy link
Contributor Author

Yea thinking about making this backwards compatible would probably involve some additional interfaces that allow you to specify which ssl method to use, this may help answer your question. I'm open to help take that on. I imaging it would be similar to the change to http listeners translated for network connectors

@softprops
Copy link
Contributor Author

going to close this in favor of a more flexible solution for NetworkConnectors in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants