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

Update password generation to support PS Core #1476

Closed
wants to merge 1 commit into from

Conversation

arnydo
Copy link

@arnydo arnydo commented Mar 12, 2020

Signed-off-by: Kyle Parrish arnydo@arnydo.com

Fixes: #1475

Core versions of PowerShell lack support for the System.Web.dll which causes [System.Web.Security.Membership]::GeneratePassword(14, 5) to fail.

Changed the password generation method to support all current versions of PowerShell.

Description

To address the lack of support for the System.Web assembly in PowerShell Core, the password generation process has been updated from:
$password = [System.Web.Security.Membership]::GeneratePassword(14, 5)

To:
$password = -join ((33..126) * 120 | Get-Random -Count 24 | ForEach-Object { [char]$_ })

Motivation and Context

When running the script with a Core edition of PowerShell such as 6.0.0 or 7.0.0 the script runs but fails to generate a password. The script continues to completion with no way to access the UI or CLI since the password is unknown/no-set.

Here is the output of the script running in 7.0.0.

PS>.\deploy_stack.ps1
InvalidOperation: C:\Working\faas\deploy_stack.ps1:30
Line |
  30 |      $password = [System.Web.Security.Membership]::GeneratePassword(24|                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Unable to find type [System.Web.Security.Membership].

MethodInvocationException: C:\Working\faas\deploy_stack.ps1:36
Line |
  36 |          $hash = $sha256.ComputeHash([System.Text.Encoding]::UTF8.GetB …
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Exception calling "GetBytes" with "1" argument(s): "Array cannot be null. (Parameter 'chars')"

MethodInvocationException: C:\Working\faas\deploy_stack.ps1:38
Line |
  38 |          $secret = [System.BitConverter]::ToString($hash).Replace('-',|          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Exception calling "ToString" with "1" argument(s): "Value cannot be null. (Parameter 'value')"

Attempting to create credentials for gateway..
  • I have raised an issue to propose this change (required)
  • My issue has received approval from the maintainers or lead with the design/approved label

How Has This Been Tested?

This has been tested in various versions of PowerShell, including:

  • 2.0
  • 5.1
  • 7.0.0

The result is a randomly generated password.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Signed-off-by: Kyle Parrish <arnydo@arnydo.com>
@burtonr
Copy link
Contributor

burtonr commented Mar 12, 2020

Just ran that command on my Windows OS PowerShell 5.1 and got this:

e0+vOe>Qk[}eW0j't!J8Z\:P

Could you test and verify that the special characters in the password work with the OpenFaaS Gateway?

for reference, the bash version generates an alpha-numeric password:

186b39e9905c11120d2d04a04b6d10b141c53229e93759e8403eec5071f0204e 

@arnydo
Copy link
Author

arnydo commented Mar 12, 2020

The special characters worked for me.

I did notice that the PowerShell script has the parameter -noHash which will return the string with special characters. By default, it will return the lowercase alpha-numeric hash.

I was simply following the format that was already in place.

This is the logic:

 if (-Not $noHash)
    {
        $sha256 = [System.Security.Cryptography.HashAlgorithm]::Create('sha256')
        $hash = $sha256.ComputeHash([System.Text.Encoding]::UTF8.GetBytes($password))

        $secret = [System.BitConverter]::ToString($hash).Replace('-', '').toLower()
    } else {
        $secret =$password 
    }

Is there a reason this option is not available in the bash script?
Is there a reason to include -noHash?

@burtonr
Copy link
Contributor

burtonr commented Mar 12, 2020

To answer your question about why the -NoHash, the commit message here is really well written: 1694313#diff-072a5e23feb4bf57b18f78d10aa9e9cf

Based on that message, I think it would be best to maintain parity as much as possible with the bash script and generate a similar alpha-numeric password

@arnydo
Copy link
Author

arnydo commented Mar 12, 2020

I completely agree.

However, by default, it will generate the alphanumeric password.

Is that not occurring for you?

❯ .\deploy_stack.ps1
Attempting to create credentials for gateway..
[Credentials]
 username: admin
 password: 08fb751797daceeb57d709057ed212582df066721e65027c5aa0b3649bfcc417
 Write-Output "08fb751797daceeb57d709057ed212582df066721e65027c5aa0b3649bfcc417" | faas-cli login --username=admin --password-stdin

Enabling basic authentication for gateway..

Deploying OpenFaaS core services
Creating service func_nats
Creating service func_queue-worker
Creating service func_prometheus
Creating service func_alertmanager
Creating service func_gateway
Creating service func_basic-auth-plugin
Creating service func_faas-swarm

@burtonr
Copy link
Contributor

burtonr commented Mar 12, 2020

I apologize, you're right. It does work exactly as expected in context. I was running only the line you had changed independent of the rest of the deploy_stack.ps1 file...

This does work as expected generating a hashed password similar to the bash function.

Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that the approach is cryptographically equivalent.

# AE: would be nice to avoid this dependency.
Add-Type -AssemblyName System.Web
$password = [System.Web.Security.Membership]::GeneratePassword(24,5)
$password = -join ((33..126) * 120 | Get-Random -Count 24 | ForEach-Object { [char]$_ })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not keen on this approach vs the original Web security API call.

Can you look at whether there is an equivalent in .NET Core that can be used?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, however, there does not seem to be a comparable alternative built in. Based on the article here it looks like the GeneratePassword function may resemble the following:

Function GeneratePassword ([int]$Length) {
    Add-Type -AssemblyName System.Web
    $CharSet = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789{]+-[*=@:)}$^%;(_!&#?>/|.'.ToCharArray()
    #Index1s 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456
    #Index10s 0 1 2 3 4 5 6 7 8
 
    $rng = New-Object System.Security.Cryptography.RNGCryptoServiceProvider
    $bytes = New-Object byte[]($Length)
 
    $rng.GetBytes($bytes)
 
    $Return = New-Object char[]($Length)
 
    For ($i = 0 ; $i -lt $Length ; $i++) {
        $Return[$i] = $CharSet[$bytes[$i]%$CharSet.Length]
    }
 
    Return (-join $Return)
 
}

The result would resemble:

GeneratePassword -Length 24

$@Unw_C6xQ+W*YBo#HEFBJv2

Would you be more comfortable adding this function to the deploy script?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts @alexellis ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm happier with the original suggestion in the linked issue under generatePassword

Is there any reason why you are using Swarm and not Kubernetes?

You can also use Git Bash to get around this on any Windows system.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexellis ,

If you would like to support PowerShell core then I think the only option is the function I referenced here. I believe it to be just as secure as the original .Net class that is being used.

I am currently using Swarm as that is what we have deployed.

Sure, there are other workarounds, but I am simply trying to address the situation where PowerShell Core may be used (Windows, Linux, OSX). Since there is already support for some PowerShell versions I wanted to expand on that. If you don't agree or find it beneficial then I will discontinue working on a solution.

@alexellis alexellis closed this Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PowerShell Core editions cannot properly run deploy_stack.ps1
3 participants