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

get rid of toxic OP_ALL option, disable SSLv3 by default #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

@lkarsten
Copy link
Contributor

Needs review.

@azet
Copy link
Author

azet commented Jun 16, 2015

@lkarsten

I see that this is also configurable right now with https://github.com/azet/hitch/blob/936b761e8c2d21a577cfc50997412e4605404653/src/hitch.c#L783.

I'm not sure this should be an option at all at this point in time. We should migrate away from SSLv3 ASAP, some CDNs like CloudFlare have already chosen not to negotiate v3 any more (https://blog.cloudflare.com/sslv3-support-disabled-by-default-due-to-vulnerability/). SSLv3 is really real-world-non-academic breakable by now. v3 also does not support SNI.

@lkarsten
Copy link
Contributor

Hi, thanks for your input.

We don't do SSLv3 in the default configuration, for the same reasons you quote.

The reason for it being there is any remaining IE6 clients out there. Unless I see some numbers saying they are just about gone, I think having it around makes sense.

@azet
Copy link
Author

azet commented Jun 18, 2015

@lkarsten

Sure, I agree. What about OP_ALL? I can commit a change that only removes that option.

@azet
Copy link
Author

azet commented Jul 25, 2015

@lkarsten Any update on that?

@azet
Copy link
Author

azet commented Sep 9, 2015

ping.

@lkarsten
Copy link
Contributor

We haven't had time to look at this. There is major changes being done on hitch right now, and we'll pick up this (and other PRs) when we are done with that. Sorry for the delay.

@daghf
Copy link
Member

daghf commented May 31, 2016

Hi @azet

Sorry for not getting back to you earlier on this one.

Regarding SSL_OP_ALL, I do agree with the points made in the article you linked, but I don't think the proper solution is to simply remove it.

The problem is rather the fact that OpenSSL added bits to SSL_OP_ALL that turned off seemingly important security features. A better way to safeguard against this would be to set the options set by SSL_OP_ALL explicitly, to make sure we don't accidentally add new workarounds when a new openssl version is out.

Thoughts on this?

@azet
Copy link
Author

azet commented Jun 1, 2016

Not all of these options are really required these days, some of them might even have negative side-effects. Nginx seems to make them configurable for example: https://github.com/nginx/nginx/blob/master/src/event/ngx_event_openssl.c#L210

IIRC: the same is the case with Apache. Some projects rely on the openssl.conf

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.

3 participants