Enable WebRequestPSCmdlet to not validate HTTPS certificates - #2006

Hi Florian Feldhaus (@ffeldhaus), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

I would suggest a parameter name of either NoCertificateCheck or SkipCertificateCheck as those parameter prefixes are more common than Ignore.... When naming a parameter, I often look at the output of this command:

(gcm -type Cmdlet).ParameterSets.Parameters | Group Name | Sort Name 

Otherwise, 👍

I would suggest to use -SkipCertificateCheck to be consistent with similar parameters: -SkipCACheck, -SkipCNCheck, and -SkipRevocationCheck.

+1 to Aleksandar Nikolić (@alexandair). Great PR, Florian Feldhaus (@ffeldhaus)!

I renamed the parameter to SkipCertificateCheck as suggested. I'm in clarification with my employee regarding the contribution license agreement and will sign it as soon as possible.

Can you add a test to powershell\test\powershell\Modules\Microsoft.PowerShell.Utility\WebCmdlets.Tests.ps1?

Awesome work Florian Feldhaus (@ffeldhaus) as for the test would something like the following be sufficient

It "Invoke-WebRequest validate skipcertificatecheck option" {

    Invoke-WebRequest -Uri 'https://expired.badssl.com' -SkipCertificateCheck | Should Not Throw
}

or

It "Invoke-WebRequest validate skipcertificatecheck option" {

    Invoke-WebRequest -Uri 'https://expired.badssl.com' -SkipCertificateCheck  | Select-Object -ExpandProperty StatusCode | Should Be 200
}

and the equivalent Invoke-RestMethod under the appropriate describe block.

James Truher (@JamesWTruher) Can you comment on Gavin Eke (@GavinEke)'s test recommendation? Is it ok for the tests to depend on an internet resource?

Any news on this getting accepted? Would fix some issues for us.

We'd like to see tests added and the CLA signed.
Some other issues:

• The parameter will exist in PowerShell Core and Windows PowerShell, but the implementation is specific to PowerShell Core. This needs to be addressed first, ideally by adding an implementation for Windows PowerShell.
• We may want multiple parameters, e.g New-PSSessionOption has -SkipCACheck, -SkipCNCheck, and -SkipRevocationCheck.

Florian Feldhaus (@ffeldhaus), Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

Jason Shirk (@lzybkr) I signed the CLA and added a test. I'm not sure if the tests get executed by running Start-PSPester (or travis or appveyor) as I couldn't find them in the console output. Could you please share more details what is required to implement the parameter for Windows PowerShell?

By default, Start-PSPester runs tests tagged CI. The web tests are tagged Feature because they are slow and we don't want them running in CI - that's why you're not seeing the test in the output. You can run Start-PSPester -Tag Feature to make sure you run your tests.

I can't really provide specific guidance on how you can implement this functionality on Windows PowerShell - but you should read this to get started.

Francisco Gamino (@Francisco-Gamino) ported these commands to PowerShell Core, so he might be able to help with specifics on Windows PowerShell.

Are there any chances this will get merged before the next release? What needs to be done to merge this pull request?

We haven't closed on whether or not it's acceptable to add this parameter to PowerShell Core but not Windows PowerShell. It sounds like people want this capability in Windows PowerShell as well, so we'd prefer that, but if not, you'll need to add #if CORECLR around the parameter so that Windows PowerShell doesn't have a parameter with no implementation.

I was also hoping to see some discussion around matching the options of New-PSSessionOption - those options most likely exist because they map nicely to wsman options - so it might be more of a question on how useful it is to disable all checks versus certain checks.

I would appreciate it, if this would also be included in Windows PowerShell. Especially for connecting to new REST endpoints it's often necessary to skip the certificate checks. I have several Cmdlets which are able to upload a new certificate via REST, but first I need to be able to accept the self-signed certificate.

In my opinion it would be better to first include the SkipCertificateCheck parameter before implementing other parameters. The SkipCertificateCheck parameter is a generic solution whereas -SkipCACheck, etc. are only useful for specific scenarios.

In the end the most important point is, that it's currently not possible to ignore certificates in PowerShell Core as the workaround available in Windows PowerShell (via [System.Net.ServicePointManager]) is not working and thus breaking lot's of Cmdlets which need to be run against REST endpoints with e.g. invalid or self signed certificates.

For what it's worth, I would vote against implementing this pull request. It's too simplistic --and dangerous-- to just have a switch that completely disables validation checking.

The vast majority of the time, the right thing to do is to just disable checking the CA chain (because it's just a self-signed cert), and not disable validation completely. The rest of the time, the right thing to do is to white-list the specific certificate and url combination.

Additionally, this not only disables the check on the initial request, it also automatically disables checking on any additional requests (i.e. if you're redirected, or download additional resources), so if you're hitting an internal website that uses a self-signed cert but then downloads additional content from CDNs with valid certs, you're needlessly disabling that validation, etc...

Additionally, you really should solve it the same way on all the web cmdlets, including Export-ODataEndpoint, etc.

I wrote a wrapper for these things a couple of years ago. It's a little complicated. In fact, it probably goes a little too far in the other direction (too many knobs). But there has to be a better solution than all or nothing.

I partly disagree with #2006 (comment) from Joel Bennett (@Jaykul). The functionality of Invoke-WebRequest and Invoke-RestMethod is similar to curl and on Windows curl is even an alias for Invoke-WebRequest. With curl its possible to ignore certificates with the -k or --insecure parameter. For compatibility reasons, it should be implemented in the same way in PowerShell.
We could additionally implement checks as suggested in #2006 (comment) by Jason Shirk (@lzybkr) for each scenario (e.g. skip CA check, skip host check, skip expiration check, ...), but that would introduce many more parameters for something which shouldn't be done for anything outside of dev/test environments.
Also I don't see an issue with disabling the check for any additional request done as part of the Invoke-WebRequest command. The behaviour should be consistent with other Cmdlets. If I use a parameter to skip the certificate check, it should be done for everything the Cmdlet does and not for parts of it.
The only thing I think would be useful is a warning message, that skipping certificates shouldn't be done for security reasons (e.g. for those PowerShell users who try to access an HTTPS page where a certificate check exception is thrown and then just ignore the certificate instead of investigating why it fails).

[regarding curl] For compatibility reasons, it should be implemented in
the same way in PowerShell.

I don't think this is a good argument when balancing against security
concerns. Otherwise--yes, I would agree.

I vote for Joel's points about use the most-right solution. I believe most
people want the user-facing behavior of "skip validation": allowing self
signed certs to be used. The fact that skipping the CA chain will achieve
this end while still performing some security checks means it's a better
workaround (that I would use often, myself).

If we were designing a feature for a layperson, we'd call it
"-AcceptTheDamnCert", and nobody would care how it's implemented. :)

On Fri, Sep 30, 2016, 3:13 AM Florian Feldhaus [email protected]
wrote:

I partly disagree with #2006 (comment)
#2006 (comment)
from Joel Bennett (@Jaykul) https://github.com/Jaykul. The functionality of
Invoke-WebRequest and Invoke-RestMethod is similar to curl and on Windows
curl is even an alias for Invoke-WebRequest. With curl its possible to
ignore certificates with the -k or --insecure parameter. For
compatibility reasons, it should be implemented in the same way in
PowerShell.
We could additionally implement checks as suggested in #2006 (comment)
#2006 (comment)
by Jason Shirk (@lzybkr) https://github.com/lzybkr for each scenario (e.g. skip CA
check, skip host check, skip expiration check, ...), but that would
introduce many more parameters for something which shouldn't be done for
anything outside of dev/test environments.
Also I don't see an issue with disabling the check for any additional
request done as part of the Invoke-WebRequest command. The behaviour
should be consistent with other Cmdlets. If I use a parameter to skip the
certificate check, it should be done for everything the Cmdlet does and not
for parts of it.
The only thing I think would be useful is a warning message, that skipping
certificates shouldn't be done for security reasons (e.g. for those
PowerShell users who try to access an HTTPS page where a certificate check
exception is thrown and then just ignore the certificate instead of
investigating why it fails).

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#2006 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABIMjgWTVUyMhxWBAlzjB-uAze3sUZAks5qvLZ4gaJpZM4JpjJC
.

We decided to take this change as is without requiring the corresponding changes to Windows PowerShell.

Any reason why this is merged but not available yet? I really hate having to break into CYGWIN from POWERSHELL to do things...

@bgoldman69 this has been available for sometime in our alpha releases

Has this been pulled from beta releases then? I'm using the latest beta (7) and there is no SkipCertificateCheck for Invoke-RestMethod in that version...

It's working for me with PowerShell Beta 7 on Mac OS X

Florians-MBP:~ ffeldhaus$ powershell 
PowerShell v6.0.0-beta.7
Copyright (C) Microsoft Corporation. All rights reserved.

PS /Users/ffeldhaus/development> Invoke-RestMethod -SkipCertificateCheck -Uri https://expired.badssl.com/    

Works for me using Beta 7 on Windows 10 (1703)

I stand corrected. It's just missing from the get-help output and thus I assumed it wasn't there.

Stephane Bourdeaud (@sbourdeaud) You can see latest docs in https://github.com/PowerShell/PowerShell-Docs repo. Feel free open Issue there if the parameter isn't documented.

Done