Creating a portable encrypted credential store

Question

There have been a few questions over on SO recently about securely storing credentials in Powershell scripts. Because ConvertFrom-SecureString uses DPAPI it makes the output only valid for that user, which makes creating credentials for service accounts etc. occasionally awkward.

I have had this script lying around for a while, that creates a private key for ConvertFrom-SecureString stores that and the password in a custom object, and then encrypts the whole thing with a self signed certificate.

While it all works perfectly well, I've always been a little nervous about how it creates the certificate. While I (think I) understand what the parameters to New-SelfSignedCertificate are doing I'm still not convinced it is configured in the most appropriate / secure way (obviously after this, the certificate is exported / deleted / secured / imported where needed).

For starters, I know backticking is generally frowned upon, but I prefer it because it aids readability.

This is the main function I'm concerned with. Would a different provider be 'better', should I be using elliptic curve, is the concept solid. etc.

#Requires –Version 5

function New-Certificate
{
    [CmdletBinding()]
    Param
    (
        [Parameter(Mandatory=$true)]
        $CertificateSubject
    )

    New-SelfSignedCertificate -TextExtension @("2.5.29.37={text}1.3.6.1.4.1.311.80.1") `
                            -Provider "Microsoft Enhanced RSA and AES Cryptographic Provider" `
                            -DnsName $CertificateSubject `
                            -CertStoreLocation  "Cert:\CurrentUser\My" `
                            -KeyExportPolicy ExportableEncrypted `
                            -KeyUsage DataEncipherment `
                            -KeyUsageProperty All `
                            -KeyLength 2048 
}
# New-Certificate -CertificateSubject "My Test Certificate"

From there there are three functions, that I'd be curious of your thoughts on.

For a simple export function, I considered putting a path checker there. If it fails, then it is being used interactively, so it's just easier to fix as it's called.

function Export-PublicCredentials
{
    [CmdletBinding()]
    Param
    (
        [Parameter(Mandatory=$true)]
        $CertificateSubject,
        [Parameter(Mandatory=$true)]
        $filePath
    )

    $Certificate = (Get-ChildItem Cert:\CurrentUser\My\ `
                    -DocumentEncryptionCert | Where-Object {$_.Subject -match $CertificateSubject} )

    Export-Certificate -Cert $Certificate -FilePath $filePath 
}

Then the real part, create a key, store the credential, wrap it all up in a cert:

function Encrypt-Credentials
{
    [CmdletBinding()]
    Param
    (
        [Parameter(Mandatory=$true)]
        $CertificateSubject
    )
    $Credential = Get-Credential 

    $key = New-Object byte[](32)
    $rng = [System.Security.Cryptography.RNGCryptoServiceProvider]::Create()
    $rng.GetBytes($key)

    $SecureStringWithKey = $Credential.Password | ConvertFrom-SecureString -Key $key 

    $thumbprint = (Get-ChildItem Cert:\CurrentUser\My\ `
                    -DocumentEncryptionCert | 
                     Where-Object {$_.Subject -match $CertificateSubject} ).Thumbprint

    $object = (New-Object pscustomobject | 
        Add-Member -PassThru NoteProperty -Name "Key" -Value $key | 
        Add-Member -PassThru NoteProperty -Name "Username" -Value $Credential.UserName |  
        Add-Member -PassThru NoteProperty -Name "SecureStringWithKey" -Value $SecureStringWithKey 
    )
    $json = $object | ConvertTo-Json

    $EncryptedKey = Protect-CmsMessage -Content $json -To $thumbprint

    Write-Output $EncryptedKey 
}

$EncrypteString = Encrypt-Credentials -CertificateSubject "My Test Certificate" 

Finally, what would be the most called parameter, the decryption. The idea here is that the encrypted string is put somewhere accessible (A HTTP share, or a local disk etc) the encrypted string is decrypted and deserialised and then you get a credential object, as well as the key to pass back to ConvertTo-SecureString to get the PSCredential object back.

function Decrypt-Credentials
{
    [CmdletBinding()]
    [OutputType([System.Management.Automation.PSCredential])]
    Param
    (
        [Parameter(Mandatory=$true)]
        $EncryptedCredentials
    )

    $AuthObject = (Unprotect-CmsMessage -Content $EncryptedCredentials | ConvertFrom-Json)

    $secureStringPassword = $AuthObject.SecureStringWithKey | 
                    ConvertTo-SecureString -Key $AuthObject.Key.value

    $cred = new-object -typename System.Management.Automation.PSCredential `
             -argumentlist $AuthObject.Username, $secureStringPassword

    Write-Output $cred 

}

#Decrypt-Credentials -EncryptedCredentials $EncrypteString

Answer 1

I don't know enough about handling credentials to know whether the overall concept is sound, so I will ignore that aspect. I do have the following suggestions regarding the details of your code.

Use Splats Instead of Backticks

$splat = @{
    TextExtension = @("2.5.29.37={text}1.3.6.1.4.1.311.80.1")
    Provider      = "Microsoft Enhanced RSA and AES Cryptographic Provider"
    DnsName       = $CertificateSubject
    CertStoreLocation = "Cert:\CurrentUser\My"
    KeyExportPolicy   = 'ExportableEncrypted'
    KeyUsage          = 'DataEncipherment'
    KeyUsageProperty  = 'All'
    KeyLength         = 2048 
}
New-SelfSignedCertificate @splat

This has the following advantages over backticked named parameters:

• can be converted to a parameters object and piped to the command with minimal alterations
• $splat key/value pairs can be conditionally added and removed
• less typing to achieve both nice layout and correctness
• satisfies the "no-backticks" pedants

Try to Start New Lines on |

$Certificate = Get-ChildItem Cert:\CurrentUser\My\ -DocumentEncryptionCert | 
    Where-Object {$_.Subject -match $CertificateSubject}

This might look a bit quirky for just two commands in the pipeline, but it expands to many commands well:

$folderNames = Get-ChildItem |
    ? { $_.Name -like '*c*' } |
    Sort { $_.LastWriteTime } |
    Select -First 2 |
    Sort { $_.CreationTime } -Descending |
    % Name

Separate Parameters with Whitespace

Parameters should be separated with whitespace:

Param
(
    [Parameter(Mandatory=$true)]
    $CertificateSubject,

    [Parameter(Mandatory=$true)]
    $FilePath
)

This doesn't make a big difference when you only have four lines in your Param() block. However, when your parameter and attribute count grows, the whitespace makes it much easier to see where one parameter ends and the next begins.

Adopt a Capitalization Convention

I don't recognize the capitalization convention you are using. For example, you have two parameters called CertificateSubject and filePath. PowerShell parameters we see in Microsoft-provided cmdlets seem to be CamelCaps. I would stick to that.

For my code, I use mixedCase for local variables. This way I can immediately tell when I am working with a parameter. That is important because modifying child objects of a parameter can have consequences outside of the scope.

Different projects seem to have different conventions. The important thing is that you pick one and stick to it.

Create PSObjects with @{}

Create PSObjects using a hashtable like this:

New-Object psobject -Property @{
    Key                 = $key
    Username            = $Credential.UserName
    SecureStringWithKey = $SecureStringWithKey 
}

This has much less cruft and might be faster than Add-Member.

Validate your Parameters

You should consider validating the parameters when they are passed to your functions:

Param
(
    [Parameter(Mandatory=$true)]
    [ValidateScript({$_ | Test-ValidCertificateSubject})]
    [string]
    $CertificateSubject,

    [Parameter(Mandatory=$true)]
    [ValidateScript({$_ | Test-ValidFilePath})]
    [string]
    $FilePath
)

That will catch problematic parameters early. [string] ensures that the parameter is a string (or is coerced to one). ValidateScript() allows you to use custom validation functions. You'd have to write Test-ValidCertificateSubject yourself. Test-ValidFilePath already exists.

This will Fail in Some Conditions

$Certificate = (Get-ChildItem Cert:\CurrentUser\My\ `
                -DocumentEncryptionCert | Where-Object {$_.Subject -match $CertificateSubject} )

Export-Certificate -Cert $Certificate -FilePath $filePath

$Certificate could contain an array of certificates. The call to Export-Certificate will fail if that is the case. How best to fix this is a design decision that requires some judgment. I find that arriving at a good solution is best achieved by setting up unit tests that include the problem case and trying a few different behaviors until you find the one that works best for your situation. I doubt, though, that the best behavior is the current form which produces this cryptic error (pun intended):

Export-Certificate : Cannot convert 'System.Object[]' to the type  'Microsoft.CertificateServices.Commands.Certificate' required by parameter 'Cert'. 
Specified method is not supported.