Skip to content

Use TLSv1.2 by default #800

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

Merged
merged 1 commit into from
Feb 16, 2017
Merged

Use TLSv1.2 by default #800

merged 1 commit into from
Feb 16, 2017

Conversation

orzeh
Copy link
Contributor

@orzeh orzeh commented Feb 16, 2017

Removes tricks with https.protocols system property and instructs Jersey SslConfigurator to use TLSv2 TLSv1.2 by default.

Fix #798


This change is Reviewable

@codecov-io
Copy link

Codecov Report

Merging #800 into master will increase coverage by 0.07%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #800      +/-   ##
==========================================
+ Coverage   71.51%   71.58%   +0.07%     
==========================================
  Files         303      303              
  Lines        6571     6568       -3     
  Branches      484      483       -1     
==========================================
+ Hits         4699     4702       +3     
+ Misses       1586     1580       -6     
  Partials      286      286
Impacted Files Coverage Δ
...ithub/dockerjava/core/LocalDirectorySSLConfig.java 18.18% <ø> (+1.51%)
...ava/netty/handler/FramedResponseStreamHandler.java 89.65% <ø> (+5.17%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af8ff78...1c18337. Read the comment docs.

@KostyaSha
Copy link
Member

So tls v2 or v1.2?

@orzeh
Copy link
Contributor Author

orzeh commented Feb 16, 2017

TLSv1.2 of course, my bad. I'll edit PR description.

@KostyaSha KostyaSha added this to the 3.0.8 milestone Feb 16, 2017
@KostyaSha
Copy link
Member

Ok, let's try!

@KostyaSha KostyaSha merged commit e8146ac into docker-java:master Feb 16, 2017
@orzeh orzeh deleted the tls branch February 16, 2017 21:34
@KostyaSha
Copy link
Member

@orzeh could you check other SSLConfigs? They seems using old implementation

@orzeh
Copy link
Contributor Author

orzeh commented Apr 3, 2017

@KostyaSha OK I'll take a look in a spare time.

@orzeh orzeh changed the title Use TLSv2 by default Use TLSv1.2 by default Apr 3, 2017
panuse pushed a commit to TuKangTech/docker-java that referenced this pull request Aug 20, 2017
@neandrake
Copy link

It looks like KeystoreSSLConfig also has a similar implementation of fiddling with https.protocols but wasn't updated to use TLSv1.2 by default. Should I make a new task for updating that?

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.

4 participants