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

Could not create SSL/TLS secure channel #21

Closed
PaulWalkerUK opened this issue Oct 14, 2018 · 12 comments
Closed

Could not create SSL/TLS secure channel #21

PaulWalkerUK opened this issue Oct 14, 2018 · 12 comments

Comments

@PaulWalkerUK
Copy link
Contributor

When importing this module and first trying any of the functions, I get:

PS E:\git\SpaceX> Get-SXApi
Invoke-RestMethod : The request was aborted: Could not create SSL/TLS secure channel.
At E:\git\SpaceX\SpaceX\Public\Get-SXApi.ps1:18 char:5
+     Invoke-RestMethod -Uri https://api.spacexdata.com/v3
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: (System.Net.HttpWebRequest:HttpWebRequest) [Invoke-RestMethod], WebExc
   eption
    + FullyQualifiedErrorId : WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeRestMethodCommand

To resolve this, I have to run this first (taken from https://stackoverflow.com/a/41618979/69698):

[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12

Once I've done that, it works fine for the rest of the session. I don't know if this is something particular to my machine/setup?

@lazywinadmin
Copy link
Owner

Hi @PaulWalkerUK I did not see this issue on my machine so far.

Here is my current config on Windows PowerShell 5.1
image

Here is my config on PSCore
image

@patrick-ryan1
Copy link

Hello,

I did see the same issue and was going to suggest the same change.
PS C:\windows\system32> $PSVersionTable

Name Value


PSVersion 5.1.16299.666
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.16299.666
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

Windows 10

@PaulWalkerUK
Copy link
Contributor Author

This is what I have:

PS C:\> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.17134.228
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.17134.228
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1


PS C:\> [Net.ServicePointManager]::SecurityProtocol
Ssl3, Tls

So I'm guessing that my problem is because [Net.ServicePointManager]::SecurityProtocol doesn't include Tls12. Why yours does and mine doesn't, I have no idea!! I'm on a reasonably up to date Windows 10 Home edition, but I haven't got PowerShell 6 - just the default as far I recall.

@PaulWalkerUK
Copy link
Contributor Author

While PR #22 fixes the issue, the more I think about it, the more I'm not sure that it's the best solution as it's (potentially) changing the SecurityProtocol for the rest of the Powershell session, whether it's required or not.

I've been doing some more reading on the issue, for example, here, here and here. They suggest that if you upgrade to .NET 4.7 then it should work. However, I am already on 4.7, so again, unless there's something 'wrong' with my machine (and maybe @patrick-ryan1's, etc), it suggests it's not quite that simple...

I'm thinking it might be cleaner to just attempt to make the call without any changes to settings. If it works, then fine, there's no need to interfere. Only if it fails (and it fails quickly, so it won't be adding a noticeable delay), then make the change, try the call again, then set it back to how it was in the first place, so as not to make any changes for anything else.

According to SSL Labs, api.spacexdata.com accepts TLS v1.2 and 1.3. The setting a bitwise setting. I'm inclined to suggest that if the first call fails, set the flags for both TLS 1.2 and 1.3 - then just put it back to whatever it was originally afterwards.

This would mean adding some code to each Invoke-RestMethod call. The simplest way to do this would be to extract that out to a separate private function, e.g. Get-SXData. All the other public functions could call this and pass the URI they wanted retrieving. This could also help with #2 as the new function could have the base URI (https://api.spacexdata.com/v3) and the calling functions could just pass in the rest of it.

What are your thoughts on this? Since it works OK for you, I'm OK with it if you think this is more likely an issue with my machine and that it shouldn't be up to this module to workaround it (in which case #22 should be removed)

@lazywinadmin
Copy link
Owner

Thanks for the input @PaulWalkerUK
As you mention, we could use a specific private function or create a module setting (store in a script variable) that all the functions use.

  • We can perform a connection test in the psm1 to validate if the SecurityProtocol need to be updated and adjust the module settings in consequence.

In other words:

Thoughts? (Hope it's clear, sorry for my english 🤢 )

@PaulWalkerUK
Copy link
Contributor Author

I agree that doing the check on module import is a good idea. At what point are you thinking of making the change? I was thinking of doing doing it immediately before each API call then resetting it back again afterwards (so as not to leave anything changed for longer than necessary) - is this your thinking as well?

@lazywinadmin
Copy link
Owner

Could something like this works ?
This does not fail on my box

Not sure if this create other issues...

#Get public and private function definition files.
$Public = @(Get-ChildItem -Path $PSScriptRoot\Public\*.ps1 -ErrorAction SilentlyContinue)
$Private = @(Get-ChildItem -Path $PSScriptRoot\Private\*.ps1 -ErrorAction SilentlyContinue)

#Dot source the files
Foreach ($import in @($Public + $Private))
{
	TRY
	{
		. $import.fullname
	}
	CATCH
	{
		Write-Error -Message "Failed to import function $($import.fullname): $_"
	}
}

# Export all the functions
Export-ModuleMember -Function $Public.Basename -Alias *

# Set Powershell to use TLS v1.2 (minimum supported by SpaceX API)
#[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12
# LOGIC TO TEST THE SECURITY PROTOCOL
Try{
    Get-SXAPI
 }catch{
    # Make all the SecurityProtocol available
    $AllProtocols = [enum]::getvalues([System.Net.SecurityProtocolType])#[System.Net.SecurityProtocolType]'Ssl3,Tls,Tls11,Tls12'
    [System.Net.ServicePointManager]::SecurityProtocol = $AllProtocols
 }

@PaulWalkerUK
Copy link
Contributor Author

My problem with changing the setting during import is that it means the module has changed the setting for anything else that happens in the PowerShell session. If you look at [System.Net.ServicePointManager]::SecurityProtocol before and after importing the module it could be different. To minimise potential impact on anything outside of this module, I think it should only be changed for the duration of the API call then reset.

I've added some example Pester tests in a demo branch - see commit PaulWalkerUK/SpaceX@87371c6. These changes (only a demo - I'm not suggesting they get used as they are 😄) check what the SecurityProtocol is before the module is loaded then tests are used after the module import and again after the API calls to verify the SecurityProtocol isn't changed. I think tests such as these should always pass, to verify that this module isn't making changes to the wider session. The tests will only work properly in a new PowerShell session

I like your idea of checking during the import to see if the change is required, but if it is, then I think we need to change it before each API call then reset it again afterwards.

@lazywinadmin
Copy link
Owner

lazywinadmin commented Oct 20, 2018

Ok i see what you mean, I guess we won't have a choice, we'll need to revert back after each call.
Thanks for the Examples/Test, this is helpful I think

After searching a bit more, I see PSCore can do the following, but we can't assume people will just use PSCore

Invoke-RestMethod -Method POST -Body $Body -Uri $URI -SslProtocol Tls12

@PaulWalkerUK
Copy link
Contributor Author

I've added a new commit to my demo branch PaulWalkerUK/SpaceX@f9a83cd. During the module import, it makes a call to Get-SXApi. If it fails, then set a flag. The functions then need to check this flag before invoking the API. If it's set, then add Tls12 to the SecurityProtocol beforehand then reset afterwards.

This works on my machine and allows my Pester tests to pass. It should work for you because the initial call during module import will succeed, so the flag won't be set therefore your SecurityProtocol won't be touched.

(This is only proof-of-concept code 😉)

@PaulWalkerUK
Copy link
Contributor Author

In commit PaulWalkerUK/SpaceX@fce98b6, I added the code to Get-SXCompany for setting the SecurityProtocol. This would need adding to every function...

Instead of that, in commit PaulWalkerUK/SpaceX@a71e722, I extracted the boiler plate code out into a new private function Get-SXData. The public functions now just need to call that and pass the relevant part of the URI as a parameter. This also addresses #2. (Only implemented this in Get-SXApi and Get-SXCompany for examples).

(This is only proof-of-concept code...)

What do you think? I'm happy to work this up into a proper PR if you're happy with the approach

@lazywinadmin
Copy link
Owner

Ah yep i like the approach, sound good to me. Thanks Paul!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants