Powershell: Add cryptographic protocol parameter to web cmdlets.

[thezim]

Add parameter to all web cmdlets to specify cryptographic protocol such SSL3, TLS1, TLS11, TLS12, etc.

[iSazonov]

/cc @markekraus Have you interest to grab this?

[markekraus]

@iSazonov can you add the Area-Cmdlets-Utility Label to it?

I'll add it to my list of things to look into and leave it up for grabs if someone else has the bandwidth before I can get to it.

[thezim]

@markekraus seems straight forward. I'll look in to it.

[markekraus]

@thezim Is the idea that outside of the provided crypto-proto(s) it will return an error? or to "best effort" the provided protocol?

One concern will be testing it. We need a separate port for each crypto-proto. WebListener will need to be expanded to include those ports and protocols. I'm not sure what all Kestrel can handle, but I believe at-least the ones you listed are available.

[thezim]

@markekraus yes, anything outside of System.Net.SecurityProtocolType would error, ServicePointManager only accepts those. I've already modified WebListener to accept a new protocol argument, I didn't see the need to add an additional port as specifying the protocol to Kestrel was easy. New controller and view was added as well for testing feedback.

[iSazonov]

We should follow CoreFX SecurityProtocolType - the parameter should be the type.
It seems we can use one port for tests.

[markekraus]

@thezim The reason we would want separate ports is for ensuring a protocol. If you have some other way of ensuring a the parameter when set to TLS12 is erroring when SSL3 is used, then that's fine.

Since ServicePointManager is persistent, we should implement on HttpClientHandler.SslProtocols instead. If users want to set their own persistent protocols, they can research those APIs, but if we have some terminating exception in the middle of call where we set ServicePointManager.SecurityProtocol, the user will be left with subsequent calls exhibiting unexpected behavior.

[iSazonov]

What scenarios do we want address?
Current browsers behavior is enable/disable protocol, no fallback to low version.

[thezim]

@iSazonov if you're at the point of explicitly specifying a protocol then we should let it fail with no fallback.

[markekraus]

@iSazonov the problem is that ServicePointManager affects more than just the web cmdlets and setting it can result in unintended consequences with other functions/cmdlets/modules even for things that are not doing HTTP requests. That kind of permanence may be OK in a scripting scenario, but will be terrifying in a console scenario. In Windows PowerShell this is already a pain.

One thing I would like to address for 6.1.0, is session-persistent settings for the web cmdlets only. That way it can be treated more like "browser" settings and less like "platform" settings. It would include these security protocols and the certificate validation callback (maybe even some predefined callbacks to use).

[iSazonov]

@markekraus Thanks for clarify! I believe we should get a conclusion from @powershell-committee.

/cc @SteveL-MSFT

[markekraus]

@iSazonov I think that can be tracked elsewhere. For this particular issue, so long as we implement on per-call through HttpClientHandler.SslProtocols it should be fine.

I'm just opposed to tickling other parts of .NET when that behavior is not apparent to the user. Adding session-persistent settings to the web cmdlets is probably too tall an order for 6.0.0. It's tied up with moving to a session persistent HttpClientHandler instead of a per-call instance. We would also want to wrap that implementation to handle the inevitable changes in CoreFX. And then we'd need to expose cmdlets for setting them as well as per-cmdlet overrides (different settings for irm vs iwr).

Point being, if that ever does get implemented, per-call overrides for these settings would still be acceptable. so we should be able to move forward with this one absent a decision by the committee. But I welcome their feedback anyway.

[SteveL-MSFT]

A cmdlet should not make changes that affect the whole process or system, unless the cmdletis designed explicitly to do so (and the user is clear of the side effects). For the discussion on this issue, I don't see a need for Committee to review.

[iSazonov]

My question was - should we do anything now before RTM or better come back later with full solution (based on RFC)?

[markekraus]

@iSazonov My opinion is that we should. The RFC I had in mind would include per-call overrides. That would mean if this issue is implemented as a Parameter to the web cmdlets it would still fit within within the future RFC's framework. If this current issue can be accepted before 6.0.0 RTM it will at least add the desired functionality per-call through the parameter and "per-session" through $PSDefaultParameterValues. I see significant value in that.

[dragonwolf83]

I don't think a parameter for this is truly wanted or needed by most users. How many people really go into IE or Chrome and start fiddling with SSL/TLS settings? On server-side, a lot, but client-side? Client-side should just work.

I think we need to be careful about not overloading too many parameters that will end up being rarely used. We just added 3. How many more?

The larger issue is that TLS 1.2 is broken because it still default to TLS 1.0. Users are only having to set TLS settings using SecurityProtocolType as a workaround. So, the immediate need is to default to the highest TLS setting supported with proper fallback support .

Second scenario are security policies disabling SSL v3 and TLS 1.0. That sounds like a platform level setting to me in that case. If it could pick up the OS defaults, that could make it simpler to make the changes in one place.

Third scenario would be overrides where things are broken on a site. I think a separate WebSessionConfiguration that holds a bunch of settings would be useful here. It keeps the many configuration places in one place and keeps from overloading too many parameters into the web cmdlets themselves.

Out of all those scenarios, I only see the first one being the most important for 6.0. The rest need an RFC to get it right.

[iSazonov]

I think all three scenarios is important - users must be able to manage settings. The _settings_ is not just protocol.

[markekraus]

@dragonwolf83

I think we need to be careful about not overloading too many parameters that will end up being rarely used.

I understand your concern, but if you look at other command line web tools, like curl and wget, they have tons of options. This not fluff or feature creep, but the nature of HTTP being filled with bell-end use cases. The average use case is not average at all. If this were an issue requesting support for status 418 "I'm a teapot" I would agree. However, forcing the protocol on per-call basis is standard feature of most all CLI web tools.

How many more?

I don't think a slippery slope fallacy is warranted when the web cmdlets already severely lag behind their CLI peers.

Second scenario are security policies disabling SSL v3 and TLS 1.0. That sounds like a platform level setting to me in that case.

Well it depends on if you are working in an environment where you need both because a legacy system does not support new crypto-protocols, but you want to ensure newer protocols for the ones that do use the newer protocols. That's where a per-call setting shines and where a platform setting fails. Then there is the issue of platform settings affecting more than just the web cmdlets and instructing uses to go to .NET settings and change this is inviting disaster for the user.

I think a separate WebSessionConfiguration that holds a bunch of settings would be useful here. It keeps the many configuration places in one place and keeps from overloading too many parameters into the web cmdlets themselves.

I think both have merit. The ability to have per-session settings (which can take the form of per-profile settings though being added to the user's profile) and per-call settings to override them all in addition to the .NET platform settings.

I think for most users they will look at the available parameters as they are easily discoverable. If they don't see the ability to set the crypto-proto they will likely just use curl or wget where these are standard features. The Web Cmdlets have a history of being useful only for very basic web tasks because they lack features that many CLI web binaries have. I believe the user base will not only be accepting of more parameters, but are desiring it or expecting it.

[iSazonov]

Have we an Issue to implement "full" wget? Maybe open if no?

[markekraus]

@iSazonov I didn't want to invite full chaos. Not everything from wget and curl makes sense or are even possible with the current web cmdlets implementation. Some of those features would require a complete replacement of HttpClient and HttpClientHandler (even AngelSharp lacks parity there and they are implementing their own TCP streams). Other features we can combine (for example, the -CertificateValidationScript allows for eliminating/combining something like 10 switches on wget...).

I think it's too late for 6.0.0, but for 6.1.0 we might look at some greater wget/curl parity and involve that in the greater discussion of how to harmonize per-call and per-session settings for the web cmdlets that do not affect the entire platform.

[iSazonov]

New Issue is good to collect and track ideas to not lose their. We shouldn't implement each Issue today :-)

[dragonwolf83]

@markekraus I understand that we need many more options, but my concern is to not rush it, let's be thoughtful and see if an option is truly needed or if a new Configuration parameter is needed that can handle various edge-cases.

The main question for this parameter is whether or not we plan to disable SSL v3 and TLS v1.0 by default in the web cmdlets. If yes, then we may need a parameter to turn it on. If not, then you don't need a parameter.

I can see arguments for both sides which is why I think a RFC is needed and wait for 6.1.0 before adding parameters we may not need. It is always harder to remove a parameter than to add it.

[iSazonov]

Enable/disable case is interesting for discussion.

[markekraus]

@iSazonov Enable/disable is interesting, but also not related to this particular issue.

@dragonwolf83 I see no benefit in delaying this particular feature which I argue should definitely be a Parameter regardless of any possible future settings changes. This is a common feature of its CLI peers. it has been requested, sought after, and asked about feature for years. It should be discoverable and not hidden away in additional settings. Inclusion of it now doesn't disrupt future plans, and if @thezim hadn't requested this, me or someone else would have.

[iSazonov]

In common scenarios, PowerShell users should not worry about the transport protocol just as they do not worry about it when using browsers.
What are the scenarios where they can't do without specifying a precise protocol? We need the exact motivation to do the right thing in the right place.

[markekraus]

In common scenarios, PowerShell users should not worry about the transport protocol just as they do not worry about it when using browsers.

Careful, equating CLI web tools to Browsers is not accurate. CLI web tools are used in automation and often that automation is subject to very stringent compliance standards.

For an interactive CLI maybe it doesn't matter. But, for compliance with secure API's ensuring the protocol per-call as well as at the platform is a common practice. Sometimes you don't have access to the platform settings (you are writing code for others) so you need to make sure your code is accessing on the compliant protocol. this way your code can be compliant an a non-compliant environment.

[markekraus]

@iSazonov @dragonwolf83 @thezim

In researching this more, I discovered that HttpClient in CoreFX is in no way affected by any settings made to System.Net.ServicePointManager. Currently, users have no external way of influencing the crypto protocols or certificate validation. HttpClient is only affected by settings made to HttpClientHenlder and currently the cmdlets are instantiating a new one on every call with no parameters exposed to make this configurable.

I personally believe the crypto related settings for the web cmdlets to be a somewhat critical feature to have. Our current implementation is either to accept the defaults (which are either too strict or too lenient depending on the context) or to dangerously trust everything and the user has no say in what crypto-proto to use. I have the PR in for the certificate validation. This issue is the other missing component.

[thezim]

@markekraus I tried implementing on HttpClientHandler and ran in to issues where it didn't seem respect the choice of protocol (macOS so far). Seemed to trying to use None when call was made after setting the property correctly, I would a have run wire shark to see what actually being sent. I then used the undesired ServicePointManager to validate my WebListener test is working property, also verified that that openssl test against WebListener was getting the desired results.

Digging down the CoreFX tree looking for a possible bug there I found this. Look like all of the handlers call ThrowOnNotAllowed at some point. Would it still be desired to use the enum SslProtocols if SSL2/3 are explicitly denied?

[markekraus]

@thezim
Real quickly I threw this together:

https://github.com/PowerShell/PowerShell/compare/master...markekraus:WebCmdletSslProtocols

to test I'm using https://www.howsmyssl.com/a/check

(irm https://www.howsmyssl.com/a/check).tls_version
[System.Net.ServicePointManager]::SecurityProtocol = [System.Net.SecurityProtocolType]::Tls11
(irm https://www.howsmyssl.com/a/check).tls_version
(irm https://www.howsmyssl.com/a/check -SslProtocol Tls11).tls_version

expected result if ServicePointManager works:

TLS 1.2
TLS 1.1
TLS 1.1

actual result:

TLS 1.2
TLS 1.2
TLS 1.1

On the macOS issue: that is known. CoreFX currently doesn't support certificate and crypto settings on cert certain SSL backends. This effects macOS and sometimes CentOS depending of the flavor of curl installed. (you can see in the tests we have pending for that reason.)

For me, I was able to get my code working on my freshly updated Sierra macOS loaner.

[markekraus]

@thezim I know you were working on this, but I'm assigning this to myself and submitting a PR as I want to try and get this added in before the release candidates block all new features.

[thezim]

@markekraus understood. My apologies, I could never get consistent test results on my macOS due to the known issues. Sorry for not handing this over sooner.

[markekraus]

@thezim All good! thanks for helping me hammer this all out and get a clearer understanding! Didn't you have something in your test code to determine what TLS version was used from the request? I could not find anything in the ASP.NET Core API to get me the TLS version. I thought I saw something for it in your repo before but when i went looking this morning I didn't see the branch from before.

[iSazonov]

https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.server.kestrel.https.httpsconnectionadapteroptions.sslprotocols?view=aspnetcore-2.0

/cc @markekraus

[thezim]

@markekraus My WebListener took a command line protocol parameter. It only listened on one protocol. Assumption was if the test code was able to successfully connect that protocol was being used. View would return that value as well. I also could not find a way to get the explicit information from the request side (controller).

[markekraus]

@iSazonov Yes, that is used for setting the option in the Kestrel server at launch. but, when a request comes in there is no way to see which protocol the request used in the controller. for example, in the program.cs it could be set as

Options.SslProtocols = SslProtocols.Tls |  SslProtocols.Tls11 | SslProtocols.Tls12

And then it would support all three versions, but in the controler I would have no way to see what protocol was used. I ended up having to add 2 more listening ports (one for TLS 1.0 and another for TLS 1.1 because the HTTPS port was already TLS 1.2). I was trying to avoid adding more listener ports,

[iSazonov]

I guess that's the only way today.