Secure Passwords

[iainbrighton]

The default virtual machine password stored by the Get-LabVMDefaults/Set-LabVMDefaults cmdlets should be implemented as a SecureString. Failure to do so will probably lead to a rejection from the PS Gallery..

[iainbrighton]

I'm thinking that dropping the -Password parameter from the Set-LabVMDefaults cmd completely. Specifying the password on the Start-LabConfiguration (and associated) cmdlet(s) would keep the password from being easily discoverable on disk and easily support the [PSCredential] type.

As a side note; it's currently very easy to specify a different password in a configuration to the actual local administrator password that is currently set during the SYSPREP process. I think it would be beneficial to have to specify the local admin password when creating a lab/VM.

[csandfeld]

Sound good to me. I think that would be quite intuitive as well.

[iainbrighton]

So having a -Password and/or -Credential parameter on Start-LabConfiguration, and removing -Password from Set-LabVMDefaults would be OK? It will remove the requirement of trying to store credentials securely, probably unsuccessfully!

[csandfeld]

Yes absolutely. And a Credential param would be sweet, giving the option to provide any existing credential object you might have already. Would fit my needs very well indeed.

[csandfeld]

Since it prompts for credentials, and expects a pscredential object, I am wondering if it would make more sense to:

• Name the parameter 'Credential' (to align with common standards)
• Include a helpful message and prepopulate the User name when prompting for credential

Example:

Get-Credential -Message 'Enter password to be used for the local administrator accounts' -UserName Administrator

I would be happy to make those changes and create a pull request - if you agree that is

[iainbrighton]

I'm with you on this one. However, I did play with various options, including using [System.Security.SecureString] instead of a [PSCredential].

Here's what I tried and how I ended up where we are.

• It was originally -Password [System.Secure.String]
  • I got fed up converting text to secure strings!
• I then changed it to -Credential [PSCredential]
  • This permits prompting for credential with (Get-Credential) on the parameters
  • The problem with this is, if you hit escape on the credential prompt, you end up with null credentials
• I changed the -Credential parameter to Mandatory to fix the "hitting escape" issue
  • Unfortunately it then ignores the -UserName and -Message parameters!
  • MS don't make it easy, do they?!
• I then renamed it to -Password [PSCredential]
  • I think naming it -Password better reflects that the username portion isn't used

Having said that, I'm open to changing its name back to -Credential if you still think it's worthwhile?!
On the mandatory requirement, I guess we could add a check in the begin { } block for this and throw if the PSCredential parameter is null?

I've just tested the SecureString implementation, and it does prompt?! How did I miss this [shakes head]?! Should we change the -Password parameter to a SecureString? People can use a PSCredential object if they want? We could have a parameter set if we want to cover all bases?

[csandfeld]

It's nice that SecureString does prompt, however it still does not allow for any informative text, and you will still need to do string conversion (or use [pscredential].GetNetworkCredential().Password) to supply a value for the parameter. No obvious solution here I guess.

I will see if I can come up with something clever (even though if there is something clever to do about it, you would probably have found it already)

[iainbrighton]

I'm happy to leave it as a [PSCredential] and I don't see a need to change it. The question is whether we rename the parameter back to -Credential (consistency with most other cmdlets) or leave it as -Password (more descriptive of it's actual purpose)? Choices, choices..

We could have both? If we're going to have informative Get-Credential text we would need to do a null check. Ensuring that we have a password via either would only add a couple of lines.. Thoughts?

[csandfeld]

These are thoughts rather than requirements (at the end of the day, it's your baby). But I would opt for "all of the above"

• Consistent naming
• Descriptive text
• Null check
• A SecureString password parameter

I think consistent naming and type of parameters will make it more intuitive for users.

[iainbrighton]

@csandfeld Just one more commit for today! Does the https://github.com/VirtualEngine/Lab/commits/Issue16 branch work for you?

In short it:

• Renames -Password [PSCredential] parameter back to -Credential [PSCredential]
  • Defaults to -Credential if nothing is specified
  • Should prompt for credentials with a message if not supplied
  • Will error if you push escape or specify an empty password
• Adds -Password [SecureString] parameter

I would like you to have a look before I merge it into the dev branch..

Forgot to say that this branch includes all today's commits too!

[csandfeld]

Thank you Iain, the password/credential feature works a treat, and I find it much more intuitive this time. Good job! 👍