-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add SslProtocol Support to WebCmdlets #5329
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
Changes from all commits
5de1ad2
e95b81f
b69e4ef
f8492cc
8a0c0c0
2f5c311
796569e
3228acf
b4ed8e5
e064196
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| using System.Collections; | ||
| using System.Globalization; | ||
| using System.Security; | ||
| using System.Security.Authentication; | ||
| using System.Security.Cryptography; | ||
| using System.Security.Cryptography.X509Certificates; | ||
| #if !CORECLR | ||
|
|
@@ -46,6 +47,35 @@ public enum WebAuthenticationType | |
| OAuth, | ||
| } | ||
|
|
||
| // WebSslProtocol is used because not all SslProtocols are supported by HttpClientHandler. | ||
| // Also SslProtocols.Default is not the "default" for HttpClientHandler as SslProtocols.Ssl3 is not supported. | ||
| /// <summary> | ||
| /// The valid values for the -SslProtocol parameter for Invoke-RestMethod and Invoke-WebRequest | ||
| /// </summary> | ||
| [Flags] | ||
| public enum WebSslProtocol | ||
| { | ||
| /// <summary> | ||
| /// No SSL protocol will be set and the system defaults will be used. | ||
| /// </summary> | ||
| Default = 0, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should be exactly
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure how well that works with Flags. We have to define 0 for the default here and if CoreFX decides
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| /// <summary> | ||
| /// Specifies the TLS 1.0 security protocol. The TLS protocol is defined in IETF RFC 2246. | ||
| /// </summary> | ||
| Tls = SslProtocols.Tls, | ||
|
|
||
| /// <summary> | ||
| /// Specifies the TLS 1.1 security protocol. The TLS protocol is defined in IETF RFC 4346. | ||
| /// </summary> | ||
| Tls11 = SslProtocols.Tls11, | ||
|
|
||
| /// <summary> | ||
| /// Specifies the TLS 1.2 security protocol. The TLS protocol is defined in IETF RFC 5246 | ||
| /// </summary> | ||
| Tls12 = SslProtocols.Tls12 | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Base class for Invoke-RestMethod and Invoke-WebRequest commands. | ||
| /// </summary> | ||
|
|
@@ -137,6 +167,12 @@ public abstract partial class WebRequestPSCmdlet : PSCmdlet | |
| [Parameter] | ||
| public virtual SwitchParameter SkipCertificateCheck { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the TLS/SSL protocol used by the Web Cmdlet | ||
| /// </summary> | ||
| [Parameter] | ||
| public virtual WebSslProtocol SslProtocol { get; set; } = WebSslProtocol.Default; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should ask: maybe use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the |
||
|
|
||
| /// <summary> | ||
| /// Gets or sets the Token property. Token is required by Authentication OAuth and Bearer. | ||
| /// </summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1414,6 +1414,70 @@ Describe "Invoke-WebRequest tests" -Tags "Feature" { | |
| } | ||
| } | ||
|
|
||
| Context "Invoke-WebRequest -SslProtocol Test" { | ||
| It "Verifies Invoke-WebRequest -SslProtocol <SslProtocol> works on <ActualProtocol>" -TestCases @( | ||
| @{SslProtocol = 'Default'; ActualProtocol = 'Default'} | ||
| @{SslProtocol = 'Tls'; ActualProtocol = 'Tls'} | ||
| @{SslProtocol = 'Tls11'; ActualProtocol = 'Tls11'} | ||
| @{SslProtocol = 'Tls12'; ActualProtocol = 'Tls12'} | ||
| # macOS does not support multiple SslProtocols | ||
| if (-not $IsMacOS) | ||
| { | ||
| @{SslProtocol = 'Tls, Tls11, Tls12'; ActualProtocol = 'Tls12'} | ||
| @{SslProtocol = 'Tls11, Tls12'; ActualProtocol = 'Tls12'} | ||
| @{SslProtocol = 'Tls, Tls11, Tls12'; ActualProtocol = 'Tls11'} | ||
| @{SslProtocol = 'Tls11, Tls12'; ActualProtocol = 'Tls11'} | ||
| @{SslProtocol = 'Tls, Tls11'; ActualProtocol = 'Tls11'} | ||
| @{SslProtocol = 'Tls, Tls11, Tls12'; ActualProtocol = 'Tls'} | ||
| @{SslProtocol = 'Tls, Tls11'; ActualProtocol = 'Tls'} | ||
| @{SslProtocol = 'Tls, Tls12'; ActualProtocol = 'Tls'} | ||
| } | ||
| # macOS does not support multiple SslProtocols and possible CoreFX for this combo on Linux | ||
| if($IsWindows) | ||
| { | ||
|
|
||
| @{SslProtocol = 'Tls, Tls12'; ActualProtocol = 'Tls12'} | ||
| } | ||
| ) { | ||
| param($SslProtocol, $ActualProtocol) | ||
| $params = @{ | ||
| Uri = Get-WebListenerUrl -Test 'Get' -Https -SslProtocol $ActualProtocol | ||
| SslProtocol = $SslProtocol | ||
| SkipCertificateCheck = $true | ||
| } | ||
| $response = Invoke-WebRequest @params | ||
| $result = $Response.Content | ConvertFrom-Json | ||
|
|
||
| $result.headers.Host | Should Be $params.Uri.Authority | ||
| } | ||
|
|
||
| It "Verifies Invoke-WebRequest -SslProtocol -SslProtocol <IntendedProtocol> fails on a <ActualProtocol> only connection" -TestCases @( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should have some tests were intended is multiple protocols. that should be allowed since you used a flags enum.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added. |
||
| @{IntendedProtocol = 'Tls'; ActualProtocol = 'Tls12'} | ||
| @{IntendedProtocol = 'Tls'; ActualProtocol = 'Tls11'} | ||
| @{IntendedProtocol = 'Tls11'; ActualProtocol = 'Tls12'} | ||
| @{IntendedProtocol = 'Tls11'; ActualProtocol = 'Tls'} | ||
| @{IntendedProtocol = 'Tls12'; ActualProtocol = 'Tls'} | ||
| @{IntendedProtocol = 'Tls12'; ActualProtocol = 'Tls11'} | ||
| # macOS does not support multiple SslProtocols | ||
| if (-not $IsMacOS) | ||
| { | ||
| @{IntendedProtocol = 'Tls11, Tls12'; ActualProtocol = 'Tls'} | ||
| @{IntendedProtocol = 'Tls, Tls12'; ActualProtocol = 'Tls11'} | ||
| @{IntendedProtocol = 'Tls, Tls11'; ActualProtocol = 'Tls12'} | ||
| } | ||
| ) { | ||
| param( $IntendedProtocol, $ActualProtocol) | ||
| $params = @{ | ||
| Uri = Get-WebListenerUrl -Test 'Get' -Https -SslProtocol $ActualProtocol | ||
| SslProtocol = $IntendedProtocol | ||
| SkipCertificateCheck = $true | ||
| ErrorAction = 'Stop' | ||
| } | ||
| { Invoke-WebRequest @params } | ShouldBeErrorId 'WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeWebRequestCommand' | ||
| } | ||
|
|
||
| } | ||
|
|
||
| BeforeEach { | ||
| if ($env:http_proxy) { | ||
| $savedHttpProxy = $env:http_proxy | ||
|
|
@@ -2332,6 +2396,69 @@ Describe "Invoke-RestMethod tests" -Tags "Feature" { | |
| } | ||
| } | ||
|
|
||
| Context "Invoke-RestMethod -SslProtocol Test" { | ||
| It "Verifies Invoke-RestMethod -SslProtocol <SslProtocol> works on <ActualProtocol>" -TestCases @( | ||
| @{SslProtocol = 'Default'; ActualProtocol = 'Default'} | ||
| @{SslProtocol = 'Tls'; ActualProtocol = 'Tls'} | ||
| @{SslProtocol = 'Tls11'; ActualProtocol = 'Tls11'} | ||
| @{SslProtocol = 'Tls12'; ActualProtocol = 'Tls12'} | ||
| # macOS does not support multiple SslProtocols | ||
| if (-not $IsMacOS) | ||
| { | ||
| @{SslProtocol = 'Tls, Tls11, Tls12'; ActualProtocol = 'Tls12'} | ||
| @{SslProtocol = 'Tls11, Tls12'; ActualProtocol = 'Tls12'} | ||
| @{SslProtocol = 'Tls, Tls11, Tls12'; ActualProtocol = 'Tls11'} | ||
| @{SslProtocol = 'Tls11, Tls12'; ActualProtocol = 'Tls11'} | ||
| @{SslProtocol = 'Tls, Tls11'; ActualProtocol = 'Tls11'} | ||
| @{SslProtocol = 'Tls, Tls11, Tls12'; ActualProtocol = 'Tls'} | ||
| @{SslProtocol = 'Tls, Tls11'; ActualProtocol = 'Tls'} | ||
| @{SslProtocol = 'Tls, Tls12'; ActualProtocol = 'Tls'} | ||
| } | ||
| # macOS does not support multiple SslProtocols and possible CoreFX for this combo on Linux | ||
| if($IsWindows) | ||
| { | ||
| @{SslProtocol = 'Tls, Tls12'; ActualProtocol = 'Tls12'} | ||
| } | ||
| ) { | ||
| param($SslProtocol, $ActualProtocol) | ||
| $params = @{ | ||
| Uri = Get-WebListenerUrl -Test 'Get' -Https -SslProtocol $ActualProtocol | ||
| SslProtocol = $SslProtocol | ||
| SkipCertificateCheck = $true | ||
| } | ||
| $result = Invoke-RestMethod @params | ||
|
|
||
| $result.headers.Host | Should Be $params.Uri.Authority | ||
| } | ||
|
|
||
| It "Verifies Invoke-RestMethod -SslProtocol <IntendedProtocol> fails on a <ActualProtocol> only connection" -TestCases @( | ||
| @{IntendedProtocol = 'Tls'; ActualProtocol = 'Tls12'} | ||
| @{IntendedProtocol = 'Tls'; ActualProtocol = 'Tls11'} | ||
| @{IntendedProtocol = 'Tls11'; ActualProtocol = 'Tls12'} | ||
| @{IntendedProtocol = 'Tls11'; ActualProtocol = 'Tls'} | ||
| @{IntendedProtocol = 'Tls12'; ActualProtocol = 'Tls'} | ||
| @{IntendedProtocol = 'Tls12'; ActualProtocol = 'Tls11'} | ||
| # macOS does not support multiple SslProtocols | ||
| if (-not $IsMacOS) | ||
| { | ||
| @{IntendedProtocol = 'Tls11, Tls12'; ActualProtocol = 'Tls'} | ||
| @{IntendedProtocol = 'Tls, Tls12'; ActualProtocol = 'Tls11'} | ||
| @{IntendedProtocol = 'Tls, Tls11'; ActualProtocol = 'Tls12'} | ||
| } | ||
| ) { | ||
| param( $IntendedProtocol, $ActualProtocol) | ||
| $params = @{ | ||
| Uri = Get-WebListenerUrl -Test 'Get' -Https -SslProtocol $ActualProtocol | ||
| SslProtocol = $IntendedProtocol | ||
| SkipCertificateCheck = $true | ||
| ErrorAction = 'Stop' | ||
| } | ||
| { Invoke-RestMethod @params } | ShouldBeErrorId 'WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeRestMethodCommand' | ||
| } | ||
|
|
||
|
|
||
| } | ||
|
|
||
| BeforeEach { | ||
| if ($env:http_proxy) { | ||
| $savedHttpProxy = $env:http_proxy | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is "system default"? In .Net? Os?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is environment specific and subject to implementation changes by CoreFX, thus the comment is not specific on purpose. It is
SslProtocol.None... which is terribly named for PowerShell users as it implies no SSL/TLS would be used (that's a C# specific terminology for the 0 field on flag enums).