When running in a .net 4.7 process TLS connections are only TLS1.2

[crmann1]

When using the octokit.net in a .net 4.7 process the TLS connections for the process are forced to only use TLS1.2. This also happens when running in a process which targets an earlier .net framework but is running on a .NET 4.7 runtime and the process author has set .

The reason this is happening is because applications which target / run on .NET 4.7 will now use the SystemDefault TLS settings of the OS. This is declared as 0 in the SecurityProtocolType enumeration

In the following pr #1758
ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls12;
was added to include TLS 1.2 as a list of supported protocols.

However this does not work when the SecurityProtocol is set to SystemDefault (0). Doing the OR operation causes the SecurityProtocol to be Tls12 only.

Since this property on the ServicePointManager is an appdomain wide setting this causes ALL connections in the process from this point forward on only use TLS1.2 rather than the expected TLS |TLS1.1 | TLS1.2. When the property is set to only TLS1.2 other components which talk to services which do not use TLS 1.2 will fail.

Would it be possible to make the following change, When the default TLS being used ( ServicePointManager.SecurityProtocol = SystemDefault), TLS1.2 should not be OR'd to the set of security protocols.

Since the dll targets .net 4.5.2 and the SystemDefault enumeration value is not available the check can be made as follows:

if (((int)System.Net.ServicePointManager.SecurityProtocol) != 0) { System.Net.ServicePointManager.SecurityProtocol |= System.Net.SecurityProtocolType.Tls12; }

REPRO:

Create a .net 4.7 console app
Add octokit

Console.Out.WriteLine($"Current default security protocol: {System.Net.ServicePointManager.SecurityProtocol}");

var github = new GitHubClient(new ProductHeaderValue("MyAmazingApp"));
Console.Out.WriteLine($"After GutHubClient default security protocol: {System.Net.ServicePointManager.SecurityProtocol}");

The security protocol after the instantiation of the GitHubClient should be "SystemDefault"

[ryangribble]

Hi @crmann1
Thanks for picking this up!

Sorry for the issue - it looks like an unfortunate side effect of the way later frameworks handle the default case, as obviously by "or" ing the value onto the existing setting we were attempting to mitigate old frameworks by ADDING TLS1. 2 to the existing settings! Being an app domain wide setting it's not something we did lightly however we did feel it was the best choice considering github moving to TLS1. 2 would have caused alot of octokit uses on earlier frameworks to stop working, and having each user need to discover what the problem is and then add a line of code to their app seemed pretty messy, thus we put something in the library.

Your proposed fix looks good, would you like to send it across as a PR?

[ryangribble]

I've pushed a fix for this, FYI #1936