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

Switch to TLS 1.2 by default #21

Merged
merged 2 commits into from
Apr 20, 2014
Merged

Switch to TLS 1.2 by default #21

merged 2 commits into from
Apr 20, 2014

Conversation

kylef
Copy link
Contributor

@kylef kylef commented Apr 20, 2014

No description provided.

It's 2014, no idea how this made it as a default for the past 16 years...
@jimloco jimloco merged commit 21ac28c into jimloco:master Apr 20, 2014
@jimloco
Copy link
Owner

jimloco commented Apr 20, 2014

Thanks.

@jimloco
Copy link
Owner

jimloco commented Apr 23, 2014

I didn't notice your last commit making tls1.2 the default until after I pushed. I rolled back that change. The reason is, ssl23 actually handles all of them (tls and ssl) via SSLv23_client_method(). This feature is actually document in the manual, and it's openssl's recommended default for compatibility. This can best be illustrated by setting up an openssl s_server instance only using tls1.2 (-tls1_2) and then have csocket connect to it using SSL23 and you'll see it works just fine. Making tls1.2 the default will cause it to fail on SSL3 connections, which perhaps is good on a security point of view, it is not good on a pragmatic point of view as I prefer this code to function out of the box. I updated the code comment to reflect this change so there is no further confusion.

@kylef kylef deleted the tls branch April 24, 2014 18:30
@kylef
Copy link
Contributor Author

kylef commented Apr 24, 2014

@jimloco Good catch, but SSL2 should be safe to disable. It has been known to be a vulnerable protocol for years. Check The Sorry State of SSL (9:10). Even Mozilla dropped it back in 2006.

Would you accept a PR disabling SSLv2 by default?

@jimloco
Copy link
Owner

jimloco commented Apr 24, 2014

Yeah, that seems fair to me. Should be pretty simple to implement, thanks!

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