Skip to content

Allow client to pick a specific TLS version and introduce PEM-based configuration#1167

Closed
amohtashami12307 wants to merge 4 commits intoredis:masterfrom
amohtashami12307:master
Closed

Allow client to pick a specific TLS version and introduce PEM-based configuration#1167
amohtashami12307 wants to merge 4 commits intoredis:masterfrom
amohtashami12307:master

Conversation

@amohtashami12307
Copy link
Copy Markdown
Contributor

@amohtashami12307 amohtashami12307 commented Oct 30, 2019 •
edited
Loading

Valid protocols "TLSv1", TLSv1.1", "TLSv1.2"

Make sure that:

  • You have read the contribution guidelines.
  • You have created a feature request first to discuss your contribution intent. Please reference the feature request ticket number in the pull request.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.

@amohtashami12307
allowing client to pick a specific tls version for redis connection. …
8a8aa2e 
…Valid protocols "TLSv1", TLSv1.1", "TLSv1.2"
@mp911de
Copy link
Copy Markdown
Collaborator

mp911de commented Oct 30, 2019

Thanks for your pull request. We've seen also requests to enable configuration of a custom hostname validator. It would make sense to tie this setting to SslOptions. We should come up with a more flexible approach to SSL configuration and expose more low-level interfaces to achieve customization where needed.

@amohtashami12307
Copy link
Copy Markdown
Contributor Author

amohtashami12307 commented Oct 30, 2019

Thank you for the quick response. Yes agreed. What other low-level interfaces were you referring to? like allowing the user to pick their desired cipher suits or in other words are there other things you would like to see?

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 30, 2019 •
edited
Loading

Codecov Report

Merging #1167 into master will decrease coverage by 0.07%.
The diff coverage is 30%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1167      +/-   ##
============================================
- Coverage     80.45%   80.38%   -0.08%     
- Complexity     5829     5830       +1     
============================================
  Files           418      417       -1     
  Lines         19151    19125      -26     
  Branches       2222     2213       -9     
============================================
- Hits          15408    15373      -35     
- Misses         2721     2728       +7     
- Partials       1022     1024       +2
Impacted Files Coverage Δ Complexity Δ
src/main/java/io/lettuce/core/RedisPublisher.java 80.23% <ø> (+1.47%) 5 <0> (ø) ⬇️
...ain/java/io/lettuce/core/SslConnectionBuilder.java 84.37% <0%> (-3.67%) 5 <0> (ø)
src/main/java/io/lettuce/core/SslOptions.java 62.5% <37.5%> (-6.25%) 11 <2> (+2)
...in/java/io/lettuce/core/codec/Utf8StringCodec.java 0% <0%> (-100%) 0% <0%> (-1%)
src/main/java/io/lettuce/core/LettuceFutures.java 33.33% <0%> (-39.84%) 1% <0%> (-7%)
.../io/lettuce/core/masterslave/AsyncConnections.java 88.23% <0%> (-11.77%) 5% <0%> (-1%)
.../io/lettuce/core/protocol/ReconnectionHandler.java 67.28% <0%> (-8.42%) 23% <0%> (-3%)
...e/core/masterslave/MasterSlaveTopologyRefresh.java 80% <0%> (-6%) 14% <0%> (-1%)
.../java/io/lettuce/core/masterslave/Connections.java 75.51% <0%> (-4.09%) 9% <0%> (-1%)
...e/core/resource/DefaultEventLoopGroupProvider.java 87.5% <0%> (-2.5%) 24% <0%> (+1%)
... and 26 more

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 03c7740...67d6ac4. Read the comment docs.

@mp911de
Copy link
Copy Markdown
Collaborator

mp911de commented Oct 30, 2019

Basically, all the things that are possible with SslContextBuilder or SslContext. Certificates, key and trustmanager factories, various protocol levels. SSL config can be tricky, so there is probably no one-fits-all.

@amohtashami12307
Copy link
Copy Markdown
Contributor Author

amohtashami12307 commented Nov 20, 2019

I have moved the SSLParamaters's protocols and ciphersuites into ssloptions. @mp911de could you please review this and let me know what you think ?

@mp911de
Copy link
Copy Markdown
Collaborator

mp911de commented Nov 22, 2019

I will have a look next week. Thanks a lot!

@amohtashami12307
Copy link
Copy Markdown
Contributor Author

amohtashami12307 commented Nov 22, 2019

Okay sounds good. Happy Thanks giving.

@amohtashami12307
Copy link
Copy Markdown
Contributor Author

amohtashami12307 commented Dec 18, 2019

Hi @mp911de, are there any update on this ?

@mp911de
Copy link
Copy Markdown
Collaborator

mp911de commented Dec 20, 2019

It will take me a bit more time until I get to this.

@mp911de
Copy link
Copy Markdown
Collaborator

mp911de commented Dec 25, 2019

I took a look and configuring ciphers and protocols makes sense. I will take this PR from here to polish it (e.g. use varargs instead of String and String[] overloads) and add PEM-based certificate options.

I would also like to move SslContextBuilder and SSLParameters creations into SslOptions.

@mp911de mp911de added the type: feature A new feature label Dec 26, 2019
@mp911de mp911de added this to the 5.3.0 milestone Dec 26, 2019
@mp911de mp911de changed the title allowing client to pick a specific tls version for redis connection. Allow client to pick a specific TLS version and introduce PEM-based configuration Dec 26, 2019
mp911de pushed a commit that referenced this pull request Dec 26, 2019
@amohtashami12307 @mp911de
Allow client to pick a specific TLS version for Redis connection #1167
a7c9473 
SslOptions allows configuration of protocols and ciperSuites.
mp911de added a commit that referenced this pull request Dec 26, 2019
@mp911de
Polishing #1167
891284d 
Replace String/String-array overloads with varargs method. Allow configuration of keys and certificates in PEM format. Move channel initialization into doInitialize(…) method.

Add author tags.
@mp911de
Copy link
Copy Markdown
Collaborator

mp911de commented Dec 26, 2019

Thanks a lot. That's squashed, merged and polished for 5.3.0. I also added a configuration option for PEM-based keys and certificates.

@mp911de mp911de closed this Dec 26, 2019
mp911de pushed a commit that referenced this pull request Dec 26, 2019
@amohtashami12307 @mp911de
Allow client to pick a specific TLS version for Redis connection #1167
061ed4c 
SslOptions allows configuration of protocols and ciperSuites.
mp911de added a commit that referenced this pull request Dec 26, 2019
@mp911de
Polishing #1167
7c78b79 
Replace String/String-array overloads with varargs method. Allow configuration of keys and certificates in PEM format. Move channel initialization into doInitialize(…) method.

Add author tags.
@tishun tishun mentioned this pull request Jun 18, 2025
Open
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

type: feature A new feature

Projects

None yet

Milestone

5.3.0

Development

Successfully merging this pull request may close these issues.

2 participants

@amohtashami12307 @mp911de