[SecretsManagement] Add-Secret should prompt ... secretly

[Jaykul]

Secrets need to be Secret

As a PowerShell user
When I want to add a secret
I should not enter it visibly in the console

If I call Add-Secret Password, I obviously don't want to enter the password visibly in the console, but currently the mandatory Secret parameter prompts for a plain text string. It should prompt for the secret as a SecureString.

Proposed technical implementation details

I understand you want to accept a PSCredential or even a byte array ...

But if we're prompting the user for the value of a mandatory parameter, then it's inherently text, so the prompt should use the SecureString input.

The simplest implementation of this is to just have two parameter sets, where the default one has a mandatory SecureString parameter, and the other one accepts an object so you can pass other types.

Add-Secret [-Name] <string> [-SecureString] <SecureString> [[-Vault] <string>] [-NoClobber]
Add-Secret [-Name] <string> [-Secret] <Object> [[-Vault] <string>] [-NoClobber]

Note that currently, the only option is for the user to actually write all of this:

Add-Secret Password (Read-Host Password -AsSecureString)

[PaulHigin]

I was thinking about this and realized we don't do this now for ConvertTo-SecureString:

PS> ConvertTo-SecureString -AsPlainText -Force                                        
cmdlet ConvertTo-SecureString at command pipeline position 1
Supply values for the following parameters:
String: Hello!
System.Security.SecureString

So I think we should consider the same behavior for Set-Secret. I don't know if we want to require -AsPlainText and -Force, aren't we considering removing them from ConvertTo-SecureString?

[PaulHigin]

Another option is to have a new cmdlet Add-SecretString -Name <> -Vault<>, and then have it safely prompt for the plain text string.

[Jaykul]

Why would anyone use ConvertTo-SecureString -AsPlainText -Force interactively? I've never filed a bug for that because nobody uses it interactively. Why would we, when we have Read-Host -AsSecureString which lets us specify the prompt and enter the text secretly? It's the wrong tool for that. As far as I know, the only reason it has the -AsPlainText option is for those non-interactive cases when ... well, back when we didn't have this module.

However, in this case, the input is implicitly a secure string!

That is, the only reason it's not typed that way, is that the command is using a generic [object] so it can also accept a PSCredential or a byte array, etc. Frankly, the best design might have a separate parameter set for each type it accepts, so users can discover the acceptable types!

But in any case, it's IMPOSSIBLE to enter those other parameter types at a mandatory input prompt. The only possible input in a prompted scenario is a secure string, or a string -- which the command is going to ConvertTo-SecureString 😣

I would almost consider not supporting string as values at all: in a world where PSReadLine is copying everything you type into a text file, we don't really want to encourage people to type their secrets into the prompt and thus into history and logs, etc. If they insist on doing that, they can always use Read-Host -AsSecureString 😉

[JustinGrote]

I agree with @Jaykul, it should be purposefully difficult to expose your credential to command logs, psreadline logs, etc. to discourage bad behavior.

[PaulHigin]

Yeah, these are good points. But ConvertTo-SecureString prompts for plain text string just like Get-Secret does, so it is an apt comparison. I think we need to continue to support plain text strings since it covers many access tokens. I am thinking a separate cmdlet to handle plain text strings is the way to go for Secrets Management.

[JustinGrote]

@PaulHigin ConvertTo-SecureString shouldn't be the example cmdlet here. It prompts for the secure string as a string, not the original plaintext. Try this and let me know how it works :)
ConvertTo-SecureString 'IWillErrorBecauseImNotAnEncryptedSecureString'

Get-Credential and [PSCredential]::New should be the reference for how this cmdlet should behave. In both cases you cannot pass a plaintext string to create a credential. Powershell users are comfortable and familiar with this. (I should note New-Credential lets you use a plaintext string but it shouldn't, I digress)

Further, ConvertTo-SecureString makes you also do -AsPlainText -Force to emphasize what you're doing is not smart from a security perspective.

From an implementation perspective, this is what I recommend:

1. Add-Secret should have -Secret as non-mandatory parameter
2. If -Secret wasn't provided, do a Read-Host -Prompt "Secret for $SecretName" -AsSecureString to prompt the user for the string. This will let them enter the string and not have the info shown in the console or and saved in the Powershell Logs.. This behavior should mirror how Get-Credential -Username something works.
3. If a user specifies a string or other non-secure format (basically anything that's not byte[], PSCredential, or Securestring) to -Secret, throw an error and provide guidance on how to supply a secure string (example: "Error: You cannot supply plaintext strings to this command for safety. Please omit -Secret and you will be prompted to enter the string. You can alternatively supply the string with ConvertTo-SecureString -AsPlainText -Force <YourString> but this is not recommended and may expose your secret to logs")

User still has the option for an insecure noninteractive path by doing:
Add-Secret -Name MySecret -Secret (ConvertTo-SecureString 'MySecret' -AsPlainText -Force)

Which they are already comfortable with if they've ever created a PSCredential from scratch without using Get-Credential and there's lots of documentation/familiarity already with this procedure.

Again, to emphasize very strongly:

The current implementation invites attackers to just scrape the User command History and/or PS event log for credentials and is strongly not recommended!

@SteveL-MSFT @joeyaiello for further feedback...

[Jaykul]

Yeah @JustinGrote ... there is an error for invalid types, but it's not the friendliest:

> Add-Secret -Name Bob -Secret ($pwd)
Add-Secret: Invalid type. Types supported: byte[], string, SecureString, PSCredential, Hashtable

> Add-Secret -Name Bob -Secret @{ cwd = $pwd }
Add-Secret: The object type for cwd Hashtable entry is not supported. Supported types are byte[], string, SecureString, PSCredential

Also, whatever New-Credential you're talking about ... it's not built-in 😉

Also, what @PaulHigin is saying is they've deprecated -Force in PS7, so this works:

(ConvertTo-SecureString Foo -AsPlainText ).ToPlainText()

Still, it's a special case, with a special parameter, and an anti-pattern.

[PaulHigin]

Ok, I plan to make two changes:

First rename 'Add-Secret' to 'Set-Secret'.

Second to add a new default parameter set for SecureString, so that the prompt is safe. You can still add a string type by parameter or pipeline, but prompting will default to SecureString.

PS> Set-Secret -Name MyStringToken

cmdlet Set-Secret at command pipeline position 1
Supply values for the following parameters:
SecureStringSecret: **********

# Set string secret directly
Set-Secret -Name MyStringToken -Secret $token
Set-Secret -Name MyStringToken -Secret 'MyToken'

# Set string secret via pipeline
$token | Set-Secret -Name MyStringToken -NoClobber

What do you think?

[SteveL-MSFT]

Fixed via #51