Add SslProtocols property to ConfigurationOptions #603
Conversation
| | syncTimeout={int} | `SyncTimeout` | `1000` | Time (ms) to allow for synchronous operations | | ||
| | tiebreaker={string} | `TieBreaker` | `__Booksleeve_TieBreak` | Key to use for selecting a server in an ambiguous master scenario | | ||
| | version={string} | `DefaultVersion` | (`3.0` in Azure, else `2.0`) | Redis version level (useful when the server does not make this available) | | ||
| | writeBuffer={int} | `WriteBuffer` | `4096` | Size of the output buffer | |
There was a problem hiding this comment.
line-ending issue here showing the entire table as changed?
There was a problem hiding this comment.
When you view this in a text editor, I just added more space at the end of each row so that the last set of |'s would line up since I needed more space for the one row I added. Nothing significant was changed other than those extra spaces on each row - can revert if it bothers you.
|
|
||
| return tmp; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Forgive the ignorance here - but why is this !CORE_CLR specific?
There was a problem hiding this comment.
The CORE_CLR was purely because I don't have the right tools locally to test my changes for core clr can thus couldn't even compile to see if I got everything correct. I suspect Core CLR would support the same concept but I don't have a way to validate currently.
| @@ -78,6 +90,10 @@ internal static void Unknown(string key) | |||
| ConfigChannel = "configChannel", AbortOnConnectFail = "abortConnect", ResolveDns = "resolveDns", | |||
| ChannelPrefix = "channelPrefix", Proxy = "proxy", ConnectRetry = "connectRetry", | |||
| ConfigCheckSeconds = "configCheckSeconds", ResponseTimeout = "responseTimeout", DefaultDatabase = "defaultDatabase"; | |||
| #if !CORE_CLR | |||
| internal const string SslProtocols = "sslProtocols"; | |||
| #endif | |||
There was a problem hiding this comment.
Same as comment above - what's specific to non-Core? The documentation fragmenting between the two is not an awesome distinction to have to make.
There was a problem hiding this comment.
Same as above - just didn't have a way to validate that I changed the core clr portions of the code correctly, so I left it out. Shouldn't be too hard to remove this if someone has the right build pieces locally.
|
Hmmm, I desperately need to change this over to the VS 2017 project system to make life easier on everyone. That's one reason I'm trying to go through PRs and make sure nothing broken by that move is left behind. |
|
So, I think I installed the right version of the .NET Core SDK, but there doesn't seem to be an SslStream.AuthenticateAsClientAsync method/extension that takes the SslProtocols parameter, so I am not sure we can even support this on CORE_CLR. Suggestions? |
|
@JonCole it appears to be there for me, as an example: ssl.AuthenticateAsClientAsync(host, new X509CertificateCollection(), SslProtocols.Tls11, checkCertificateRevocation: true)...so I think we can just simplify everything here and remove a lot of |
|
@NickCraver - Sounds good. Would you mind making that change when you merge the PR? |
|
@NickCraver - I went ahead and made the change to use that signature for core clr, but was still unable to build locally, so I am hoping it works now. |
|
I was going to say that |
This property allows you to configure the list of acceptable SSL/TLS Protocols for encrypted communication. Without this setting, users are required to make changes to machine wide settings, which can break other applications that are not compatible.