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

PaulHigin · 2018-11-15T17:57:28Z

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

src/System.Management.Automation/resources/RemotingErrorIdStrings.resx

adityapatwardhan · 2018-11-15T22:03:47Z

@PaulHigin Please resolve merge conflict.

Also, enable tests here for non-windows: https://github.com/PowerShell/PowerShell/blob/master/test/powershell/engine/Remoting/SessionOption.Tests.ps1

adityapatwardhan

Please enable SessionOption tests.

PaulHigin · 2018-11-16T00:07:45Z

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 · 2018-11-16T06:46:59Z

PowerShell Committee for the breaking change?

TravisEz13 · 2018-11-16T19:08:23Z

Adding the committee to review the breaking change

…o Unix_SkipCA_Fix

test/powershell/engine/Remoting/PSSession.Tests.ps1

iSazonov · 2018-11-21T03:22:02Z

test/powershell/engine/Remoting/PSSession.Tests.ps1

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


We can use new Pester features:

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

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

Sample from the repo

PowerShell/test/powershell/Language/Classes/Scripting.Classes.Attributes.Tests.ps1

Lines 270 to 274 in 43e0394

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"

TravisEz13 · 2018-11-26T21:20:43Z

src/System.Management.Automation/resources/RemotingErrorIdStrings.resx

    <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>


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

One comment about the message

test/powershell/engine/Remoting/PSSession.Tests.ps1

adityapatwardhan · 2018-11-28T00:15:04Z

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

adityapatwardhan · 2018-11-28T00:17:21Z

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

TravisEz13 · 2018-11-28T20:46:13Z

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

…PowerShell#8279)

PaulHigin added 2 commits November 14, 2018 15:55

Add SkipCA and SkipCN check requirement before allowing HTTPS connection

c163520

Merge remote-tracking branch 'upstream/master' into Unix_SkipCA_Fix

8d17632

PaulHigin requested review from BrucePay, TravisEz13, adityapatwardhan, anmenaga, daxian-dbw and mirichmo as code owners November 15, 2018 17:57

PaulHigin assigned adityapatwardhan Nov 15, 2018

PaulHigin removed request for BrucePay, adityapatwardhan, anmenaga, daxian-dbw and mirichmo November 15, 2018 17:57

PaulHigin added WG-Remoting PSRP issues with any transport layer Breaking-Change breaking change that may affect users labels Nov 15, 2018

PaulHigin requested a review from SteveL-MSFT November 15, 2018 18:00

SteveL-MSFT requested changes Nov 15, 2018

View reviewed changes

src/System.Management.Automation/resources/RemotingErrorIdStrings.resx Outdated Show resolved Hide resolved

Updated error message per CR

f3aa030

adityapatwardhan requested changes Nov 15, 2018

View reviewed changes

adityapatwardhan added the Documentation Needed in this repo Documentation is needed in this repo label Nov 15, 2018

SteveL-MSFT approved these changes Nov 15, 2018

View reviewed changes

iSazonov added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Nov 16, 2018

PaulHigin added 2 commits November 16, 2018 10:43

Add required parameter tests

a1e117d

Merge branch 'master' into Unix_SkipCA_Fix

557f385

TravisEz13 added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Nov 16, 2018

PaulHigin added 2 commits November 16, 2018 11:32

Add New-PSSessionOption to expected cmdlet list for CoreUnix

837c6f9

Merge branch 'Unix_SkipCA_Fix' of github.com:PaulHigin/PowerShell int…

76c6ed0

…o Unix_SkipCA_Fix

adityapatwardhan requested changes Nov 20, 2018

View reviewed changes

PaulHigin added 3 commits November 20, 2018 14:11

Update1

197b6eb

Add conditional to Before work

d52b894

Changes to conform to test pattern requirement

3cb1562

iSazonov reviewed Nov 21, 2018

View reviewed changes

Update to use different error check pattern

f504f35

TravisEz13 reviewed Nov 26, 2018

View reviewed changes

TravisEz13 requested changes Nov 26, 2018

View reviewed changes

Updated error message per CR

6c4f2e2

adityapatwardhan requested changes Nov 27, 2018

View reviewed changes

test/powershell/engine/Remoting/PSSession.Tests.ps1 Show resolved Hide resolved

test/powershell/engine/Remoting/PSSession.Tests.ps1 Show resolved Hide resolved

TravisEz13 approved these changes Nov 27, 2018

View reviewed changes

adityapatwardhan approved these changes Nov 28, 2018

View reviewed changes

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

iSazonov pushed a commit to iSazonov/PowerShell that referenced this pull request Nov 29, 2018

Add SkipCA and SkipCN check requirement to WinRM/OMI HTTPS connection (…

e4825ab

…PowerShell#8279)

PaulHigin deleted the Unix_SkipCA_Fix branch August 5, 2019 21:41

joeyaiello removed the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Mar 11, 2020

iSazonov mentioned this pull request Jan 16, 2021

new-psSessionOption: A parameter cannot be found that matches parameter name 'IdleTime #12604

Closed

	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"

	<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>

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

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

Uh oh!

Conversation

PaulHigin commented Nov 15, 2018

PR Summary

PR Checklist

Uh oh!

Uh oh!

adityapatwardhan commented Nov 15, 2018

Uh oh!

adityapatwardhan left a comment

Choose a reason for hiding this comment

Uh oh!

PaulHigin commented Nov 16, 2018

Uh oh!

iSazonov commented Nov 16, 2018

Uh oh!

TravisEz13 commented Nov 16, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iSazonov Nov 21, 2018

Choose a reason for hiding this comment

Uh oh!

PaulHigin Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

iSazonov Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

TravisEz13 Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

TravisEz13 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adityapatwardhan commented Nov 28, 2018

Uh oh!

adityapatwardhan commented Nov 28, 2018

Uh oh!

TravisEz13 commented Nov 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants