★ wanayoo — archive 1999 https://github.com/PowerShell/PowerShell/pull/8279Nouvelle recherche | Portail wanayoo
Skip to content
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

Add SkipCA and SkipCN check requirement to WinRM/OMI HTTPS connection #8279

Merged
merged 12 commits into from Nov 28, 2018

Conversation

@PaulHigin
Copy link
Contributor

commented Nov 15, 2018

PR Summary

OMI does not support server certificate validation, so this change is to add a requirement that -SkipCACheck and -SkipCNCheck options are specified before a connection is allowed. This way users will be aware that no server certificate validation is performed.

This only applies to PSCore6 remoting clients on non-Windows platforms, when creating a remote session to a Windows machine via WinRM\OMI.

This is a breaking change because these skip check options are now required. If the skip check options are not included then the connection will be denied.

Examples:

# On a Linux or MacOS machine
New-PSSession -Cn <WindowsMachine> -Credential $cred -Authentication Basic -UseSSL
New-PSSession : HTTPS on Unix does not support CA or CN checks. Use the PSSessionOption -SkipCACheck and -SkipCNCheck if you are certain of the server you are connecting to.
At line:1 char:1
+ New-PSSession -cn paulhig-2 -Credential $cred -Authentication Basic - ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : ResourceUnavailable: (:) [New-PSSession], PSRemotingTransportException
+ FullyQualifiedErrorId : System.Management.Automation.Remoting.PSRemotingDataStructureException,Microsoft.PowerShell.Commands.NewPSSessionCommand

$so = New-PSSessionOption -SkipCACheck -SkipCNCheck
$session = New-PSSession -Cn <WindowsMachine> -Credential $cred -Authentication Basic -UseSSL -SessionOption $so

Enter-PSSession -Cn <WindowsMachine> -Credential $cred -Authentication Basic -UseSSL -SessionOption $so

PR Checklist

@adityapatwardhan

This comment has been minimized.

Copy link
Member

commented Nov 15, 2018

@adityapatwardhan
Copy link
Member

left a comment

Please enable SessionOption tests.

@PaulHigin

This comment has been minimized.

Copy link
Contributor Author

commented Nov 16, 2018

SessionOption.Tests.ps1 contains tests for WSMan SessionOption object, which we do not expose for non-Windows platforms. But I'll add tests for New-PSSessionOption.

@iSazonov

This comment has been minimized.

Copy link
Collaborator

commented Nov 16, 2018

PowerShell Committee for the breaking change?

@iSazonov iSazonov added the CL-Engine label Nov 16, 2018

PaulHigin added some commits Nov 16, 2018

@TravisEz13

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

Adding the committee to review the breaking change

PaulHigin added some commits Nov 20, 2018

throw "No Exception!"
}
catch {
$_.Exception.ErrorCode | Should -Be $expectedErrorCode

This comment has been minimized.

Copy link
@iSazonov

iSazonov Nov 21, 2018

Collaborator

We can use new Pester features:

$exc = { & $scriptBlock } | Should -Throw -ErrorId "..." -PassThru
$exc.Exception.ErrorCode | Should -Be $expectedErrorCode

This comment has been minimized.

Copy link
@PaulHigin

PaulHigin Nov 26, 2018

Author Contributor

I like this pattern, however the ErrorId is not being checked. At least in my current version of Pester (4.4.2).

This comment has been minimized.

Copy link
@iSazonov

iSazonov Nov 26, 2018

Collaborator

Sample from the repo

It 'Empty dynamically generated set throws in C#' {
$exc = {
Get-TestValidateSet5 -Param1 "TestString1" -ErrorAction Stop
} | Should -Throw -ErrorId "ParameterArgumentValidationError,Test.Language.TestValidateSetCommand5" -PassThru
$exc.Exception.InnerException.ErrorRecord.FullyQualifiedErrorId | Should -BeExactly "ValidateSetGeneratedValidValuesListIsNull"

@@ -1670,4 +1670,7 @@ All WinRM sessions connected to PowerShell session configurations, such as Micro
<data name="CannotGetHostInteropTypes" xml:space="preserve">
<value>Host system does not have the correct version of Hyper-V schema.</value>
</data>
<data name="UnixOnlyHttpsWithoutSkipCACheckNotSupported" xml:space="preserve">
<value>HTTPS on Unix does not currently support CA or CN checks. Use the PSSessionOption -SkipCACheck and -SkipCNCheck if you are certain of the server you are connecting to.</value>

This comment has been minimized.

Copy link
@TravisEz13

TravisEz13 Nov 26, 2018

Member
Suggested change
<value>HTTPS on Unix does not currently support CA or CN checks. Use the PSSessionOption -SkipCACheck and -SkipCNCheck if you are certain of the server you are connecting to.</value>
<value>HTTPS on Unix does not currently support CA or CN checks. Use the PSSessionOption -SkipCACheck and -SkipCNCheck if you are certain you trust the server you are connecting to and the network in between.</value>
@TravisEz13
Copy link
Member

left a comment

One comment about the message

@adityapatwardhan

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

I thought -cn would flag a PSScriptAnalyzer error, but verified that it does not. So signing off.

@adityapatwardhan adityapatwardhan merged commit 0657163 into PowerShell:master Nov 28, 2018

6 of 7 checks passed

CodeFactor 11 issues fixed. 12 issues found.
Details
PowerShell-CI-linux #PR-8279-20181127.01 succeeded
Details
PowerShell-CI-macos #PR-8279-20181127.01 succeeded
Details
PowerShell-CI-spelling #PR-8279-20181127.01 succeeded
Details
PowerShell-CI-windows #PR-8279-20181127.01 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla All CLA requirements met.
Details
@adityapatwardhan

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

@PowerShell/powershell-committee I did not realize this was marked for committee review. If this does not pass review, I will revert.

@TravisEz13

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

@PowerShell/powershell-committee Also, consider if this should be back-ported to 6.1 and 6.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.