PowerShell / SecretManagement

PowerShell module to consistent usage of secrets through different extension vaults
MIT License
317 stars 46 forks source link

Multi-Thread\Runspace usage #188

Open adamdriscoll opened 2 years ago

adamdriscoll commented 2 years ago

I've realized that the secret management module is not thread safe. In PowerShell Universal, we create a new runspace when accessing secret management, but it doesn't matter because secret management maintains its own static runspace. This results in errors like this:

2021-12-16 15:15:12.669 -08:00 [ERR] Failed to read secret: Unable to get secret Executor from vault BuiltInLocalVault
System.Management.Automation.PSInvalidOperationException: Unable to get secret Executor from vault BuiltInLocalVault
 ---> System.Management.Automation.PSInvalidOperationException: The pipeline was not run because a pipeline is already running. Pipelines cannot be run concurrently.
   at System.Management.Automation.Runspaces.PipelineBase.DoConcurrentCheck(Boolean syncCall, Object syncObject, Boolean isInLock)
   at System.Management.Automation.Runspaces.RunspaceBase.DoConcurrentCheckAndAddToRunningPipelines(PipelineBase pipeline, Boolean syncCall)
   at System.Management.Automation.Runspaces.PipelineBase.CoreInvoke(IEnumerable input, Boolean syncCall)
   at System.Management.Automation.Runspaces.PipelineBase.Invoke(IEnumerable input)
   at System.Management.Automation.PowerShell.Worker.ConstructPipelineAndDoWork(Runspace rs, Boolean performSyncInvoke)
   at System.Management.Automation.PowerShell.Worker.CreateRunspaceIfNeededAndDoWork(Runspace rsToUse, Boolean isSync)
   at System.Management.Automation.PowerShell.CoreInvokeHelper[TInput,TOutput](PSDataCollection`1 input, PSDataCollection`1 output, PSInvocationSettings settings)
   at System.Management.Automation.PowerShell.CoreInvoke[TInput,TOutput](PSDataCollection`1 input, PSDataCollection`1 output, PSInvocationSettings settings)
   at System.Management.Automation.PowerShell.Invoke(IEnumerable input, PSInvocationSettings settings)
   at Microsoft.PowerShell.SecretManagement.PowerShellInvoker.InvokeScriptWithHost[T](PSCmdlet cmdlet, String script, Object[] args, Exception& terminatingError) in D:\a\_work\1\s\src\code\Utils.cs:line 1546
   --- End of inner exception stack trace ---

I'll put some synchronization code around our service but was wondering if this could be possible in secret management itself.

adamdriscoll commented 2 years ago

I think this was also causing a process crash.

image

adamdriscoll commented 2 years ago

I'm happy to create a PR for this since it's really causing some problems with my customers. What I really need is two-fold.

What I'm considering doing is providing some way to register a Runspace factory method with the PowerShellInvoker class and if it exists, then it will be used to create runspaces on every call.

If it doesn't exist, then the existing functionality will be used as to not change the behavior of the module. I have to imagine my use-case is not very widely used.

PaulHigin commented 2 years ago

Sorry for the delayed response. Yes, I think this can be accommodated. I went with static runspaces for importing/running extension vaults to minimize resource/perf issues, and also I was thinking only of single threaded use.

It seems the best way to do this is via SecretManagement configuration scoped to current user, and will require new Set/Get Configuration cmdlets. I don't know when I'll have time to do the work, but if you create a PR, I'll be happy to review.

PaulHigin commented 2 years ago

/cc: @anamnavi

JTDotNet commented 1 month ago

Would managing runspace creation as suggested resolve the issue of accessing the vault across multiple threadjobs as in the example below?

using namespace System.Collections.Generic
using namespace ThreadJob

Import-Module ThreadJob

$secretNames = @(
    [Array of the names of several secrets]
)

$storeAuthPath = [path to exported vault password]

$script = {
    param(
        [Parameter(Mandatory=$true)]
        [string]$storeAuthPath,
        [Parameter(Mandatory=$true)]
        [string]$secretName
    )
    $ErrorActionPreference = 'Stop'
    Import-Module Microsoft.PowerShell.SecretManagement
    Import-Module Microsoft.PowerShell.SecretStore

    try {
        Get-Secret -Name $secretName
    }
    catch {        
        $password = Import-Clixml -Path $storeAuthPath
        Unlock-SecretStore -Password $password -PasswordTimeout 5
        Get-Secret -Name $secretName
    }
}

$jobs = New-Object List[ThreadJob]

foreach ($secretName in $secretNames) {
    $job = Start-ThreadJob -ScriptBlock $script -ArgumentList(@($storeAuthPath,  $secretName))
    $jobs.Add($job)
}

$secret = $jobs | Receive-Job -Wait

$secret | ConvertFrom-SecureString -AsPlainText