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

remove sslv3 from server's TLS supported versions #8588

Merged
merged 1 commit into from
Oct 17, 2014

Conversation

dqminh
Copy link
Contributor

@dqminh dqminh commented Oct 15, 2014

This makes TLS1.0 the minimum fallback version for TLS and helps mitigate poodle issue

Docker-DCO-1.1-Signed-off-by: Daniel, Dao Quang Minh dqminh89@gmail.com (github: dqminh)

@ewindisch
Copy link
Contributor

Thank you! We also need this to address websockets and registry communications as well. @tiborvass has prepared an alternative patch and will send a PR shortly.

@tiborvass
Copy link
Contributor

@dqminh this is my branch, feel free to test it/pull it: https://github.com/tiborvass/docker/tree/poodle.
Didn't test the websockets yet.

EDIT: removed the websocket part since it should already be handled in the server handler.

Signed-off-by: Tibor Vass <teabee89@gmail.com>

Docker-DCO-1.1-Signed-off-by: Daniel, Dao Quang Minh <dqminh89@gmail.com> (github: dqminh)
@tiborvass
Copy link
Contributor

Thanks @dqminh !

@tiborvass tiborvass added this to the 1.3.1 milestone Oct 16, 2014
@tiborvass
Copy link
Contributor

Ping @jfrazelle @erikh @LK4D4

It'd be great if we could test all TLS aspects of this, + being critical about implications: is it okay to break people who don't have TLS1.0 (don't know who they would be) ?

The reason behind using TLS1.0 and not 1.1 or 1.2 is because Python ssl lib is so bad that they will support 1.1 and 1.2 only in December with python 2.7.9. Anyway, critics welcome.

@thaJeztah
Copy link
Member

@tiborvass you probably know this site, but https://www.ssllabs.com/ssltest/ produces quite an extensive report on SSL support on browsers and hints on how to improve.

Here's the report for registry.hub.docker.com;
https://www.ssllabs.com/ssltest/analyze.html?d=registry.hub.docker.com

@erikh
Copy link
Contributor

erikh commented Oct 16, 2014

Perhaps a flag to downgrade?

On Oct 16, 2014, at 12:27 PM, Tibor Vass notifications@github.com wrote:

Ping @jfrazelle @erikh @LK4D4

It'd be great if we could test all TLS aspects of this, + being critical about implications: is it okay to break people who don't have TLS1.0 (don't know who they would be) ?

The reason behind using TLS1.0 and not 1.1 or 1.2 is because Python ssl lib is so bad that they will support 1.1 and 1.2 only in December with python 2.7.9. Anyway, critics welcome.


Reply to this email directly or view it on GitHub.

@tiborvass
Copy link
Contributor

@ewindisch what do you think? Should we be very aggressive on this one, or find a way of customizing it now?

@dqminh
Copy link
Contributor Author

dqminh commented Oct 17, 2014

I think it should be safe to just disable sslv3 altogether. Cloudflare has a report that says sslv3 users contributes to 0.09% of their traffic ( 0.65% HTTPS ), and most of them are on windows XP ( https://blog.cloudflare.com/sslv3-support-disabled-by-default-due-to-vulnerability/). Therefore i dont think it can be overlap with the API users set.

@tiborvass
Copy link
Contributor

@dqminh ok, thanks. LGTM

Will need 1 more from core, and would like one from @ewindisch :)

@jessfraz
Copy link
Contributor

LGTM

@ewindisch
Copy link
Contributor

LGTM

@tiborvass I see this patch as implementing the floor; the widest set of crypto we might support (raising the bar above, SSLv3). I'd love if it were followed by a patch to make it customizable. Ideally, in environments where one doesn't need Python compat they'll be able to force TLS 1.2 (and we might even make that default?)

tiborvass added a commit that referenced this pull request Oct 17, 2014
remove sslv3 from server's TLS supported versions
@tiborvass tiborvass merged commit 10f7897 into moby:master Oct 17, 2014
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.

None yet

6 participants