forked from chocolatey/choco
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
(chocolatey#2902) ReplaceTLS/HTTPS handling code
After speaking with the team and reviewing .NET's best practices documentation on this, this is the solution we've come to. Being overly specific about TLS versions has its downsides so it's better to leave it to the OS. I also replaced the class that handles it with one that has a more descriptive name and properly made it a static class.
- Loading branch information
Showing
5 changed files
with
71 additions
and
56 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
63 changes: 63 additions & 0 deletions
63
src/chocolatey/infrastructure/registration/HttpsSecurity.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| namespace chocolatey.infrastructure.registration | ||
| { | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Net; | ||
| using System.Text; | ||
| using System.Threading.Tasks; | ||
|
|
||
| internal static class HttpsSecurity | ||
| { | ||
| /// <summary> | ||
| /// Resets the <see cref="ServicePointManager.SecurityProtocol"/> and <see cref="ServicePointManager.ServerCertificateValidationCallback"/> for HTTPS connections | ||
| /// in order to ensure that PowerShell scripts we run can't have a persistent effect on these settings across the whole process after they're done running. | ||
| /// </summary> | ||
| internal static void Reset() | ||
| { | ||
| // SystemDefault is used because: | ||
| // 1. Most supported operating systems default to / require TLS 1.2 anyway. | ||
| // 2. Explicitly *enabling* any less secure protocols leaves us open to TLS downgrade attacks. | ||
| // https://en.wikipedia.org/wiki/Downgrade_attack | ||
| // 3. Always *requiring* TLS 1.2 or higher on any OS version may cause issues with some folks' internal | ||
| // networks which may have older infrastructure that doesn't support it, and they won't have a way | ||
| // to make Chocolately CLI work with that infrastructure if we're forcing a TLS version they can't | ||
| // support. | ||
| // 4. Thus, the only sensible solution (since we don't want to add a configuration value for this) is | ||
| // to take the OS-level setting and use that, because if folks set their system up to use certain | ||
| // TLS versions, we should probably follow suit, regardless of what their OS version may be. | ||
| // | ||
| // See https://learn.microsoft.com/en-us/dotnet/framework/network-programming/tls for more information | ||
| // and best practices recommendation from the .NET team. | ||
| if (ServicePointManager.SecurityProtocol != SecurityProtocolType.SystemDefault) | ||
| { | ||
| "chocolatey".Log().Warn( | ||
| "{0} was set to {1}, resetting to SystemDefault.", | ||
| nameof(ServicePointManager.SecurityProtocol), | ||
| ServicePointManager.SecurityProtocol); | ||
|
|
||
| ServicePointManager.SecurityProtocol = SecurityProtocolType.SystemDefault; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| // We reset this to ensure that if a package overrides this callback during its script runs, it doesn't affect the rest of | ||
| // the actions we need to take for the duration of this process (other packages we're installing, etc.) Setting this to | ||
| // null restores the default behaviour of certificate validation instead of whatever custom behaviour has been injected. | ||
| if (ServicePointManager.ServerCertificateValidationCallback != null) | ||
| { | ||
| "chocolatey".Log().Warn( | ||
| "{0} was set to '{1}' Removing.", | ||
| nameof(ServicePointManager.ServerCertificateValidationCallback), | ||
| ServicePointManager.ServerCertificateValidationCallback); | ||
|
|
||
| ServicePointManager.ServerCertificateValidationCallback = null; | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| "chocolatey".Log().Warn("Error resetting ServerCertificateValidationCallback: {0}".FormatWith(ex.Message)); | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters