Pester icon indicating copy to clipboard operation
Pester copied to clipboard

Test script using impersonation "hangs" when intialising TestRegistry

Open mikeclayton opened this issue 6 years ago • 7 comments
trafficstars

1. General summary of the issue

When running a script which uses impersonation (e.g. issue #1340) and the impersonating user doesn't have Read/Write permissions to the registry, Pester enters an infinite loop trying to initialise the TestRegistry.

2. Describe Your Environment

Pester version     : 3.4.0 C:\Program Files\WindowsPowerShell\Modules\Pester\3.4.0\Pester.psd1
PowerShell version : 5.1.17763.592
OS version         : Microsoft Windows NT 10.0.17763.0

but note that Pester is being imported from a private folder at version 4.8.1

3. Expected Behavior

Pester should not "hang" in an infinite loop if the impersonated user cannot access the registry.

4.Current Behavior

Pester appears to hang because it enters an infinite loop trying to initialise the TestRegistry.

5. Possible Solution

6. Context

Some scripts using impersonation might appear to hang if Pester cannot initialise the TestRegistry

mikeclayton avatar Aug 12 '19 21:08 mikeclayton

To reproduce, follow steps similar to #1344 to create a basic dummyuser account with limited access, but give the account read/write access to the logged on user's AppData folder to work around the TestDrive issue in #1344.

Then, replace the New-RandomTempRegistry implementation in TestRegistry.ps1 with this to get some additional trace information:

function New-RandomTempRegistry {
    write-host "new-randomtempregistry 1"
    do {
        write-host "new-randomtempregistry 2"
        $tempPath = Get-TempRegistry
        write-host $tempPath
        $Path = & $SafeCommands['Join-Path'] -Path $tempPath -ChildPath ([Guid]::NewGuid())
        write-host $Path
        write-host "new-randomtempregistry 3"
        $result = $false
        $result = & $SafeCommands['Test-Path'] -Path $Path -ErrorAction "Stop";
        write-host "result = '$result'"
    } until (-not (& $SafeCommands['Test-Path'] -Path $Path ))
    write-host "new-randomtempregistry 4"
    & $SafeCommands['New-Item'] -Path $Path
    write-host "new-randomtempregistry 5"
}

and run the sample code from #1344:

$ErrorActionPreference = "Stop";
Set-StrictMode -Version "Latest";

Import-Module -Name .\pester\Pester -ErrorAction "Stop"; #-RequiredVersion 3.4.0
Import-Module -Name .\securityfever-2.5.0\SecurityFever -ErrorAction "Stop";

write-host ($psversiontable | ft | out-string);
write-host (Get-Module | ft | out-string);

$Cred = Get-Credential; # enter credentals for "dummyuser"
Push-ImpersonationContext -Credential $Cred -LogonType Interactive

Describe "example describe" {
}

Pop-ImpersonationContext

The output will be something like this:

Describing example describe
new-randomtempregistry 1
new-randomtempregistry 2
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester\687584ea-b769-4204-b818-17c79fba33fc
new-randomtempregistry 3
result = 'True'
new-randomtempregistry 2
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester\cd92fed6-63a7-40d6-9042-b9708b096570
new-randomtempregistry 3
result = 'True'
new-randomtempregistry 2
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester\172ca9b1-5a73-4ed9-b3cc-2a1c87959fe9
new-randomtempregistry 3
result = 'True'
new-randomtempregistry 2
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester\4f808d60-b060-4eea-9cfa-07f30732f720
new-randomtempregistry 3
result = 'True'
new-randomtempregistry 2
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester\f4c50ce1-0935-457b-bc69-ba12b06aaa8d
new-randomtempregistry 3
result = 'True'
new-randomtempregistry 2
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester
Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester\8d7b4663-7d4e-4422-9191-668c1be0caee
new-randomtempregistry 3
... etc ...

For some reason, Test-Path returns $true for Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester\* and the loop goes on indefinitely looking for guids that return $false.

mikeclayton avatar Aug 12 '19 21:08 mikeclayton

For a simpler example:

PS> Test-Path "Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester"
False

PS> Import-Module -Name .\securityfever-2.5.0\SecurityFever -ErrorAction "Stop";
PS> $Cred = Get-Credential; # enter dummy user credentials
cmdlet Get-Credential at command pipeline position 1
Supply values for the following parameters:
Credential
PS> Push-ImpersonationContext -Credential $Cred -LogonType Interactive

PS> Test-Path "Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester"
True

PS> Test-Path "Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\nonexistentkey"
True

PS> Pop-ImpersonationContext

PS> Test-Path "Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\nonexistentkey"
False

I'm not sure where the issue is (PowerShell? SecurityFever?) but Test-Path returns $true for non-existent registry keys when using impersonation, and that breaks Pester...

mikeclayton avatar Aug 12 '19 22:08 mikeclayton

That's an interesting bug. The solution would be to fix Test-Path to work correctly, but that would be difficult / impossible to do in all versions of PowerShell. Are we able to fix this by checking if the user has permissions to registry and in that case not setup TestRegistry? That seems like an okay solution. Do you have any other proposed solutions? I don't want to make it fail just because user does not have permissions, because we don't have option to disable TestRegistry and it will try to set it up for every It, no matter if the user is using TestRegistry or not.

nohwnd avatar Aug 19 '19 19:08 nohwnd

It seems that Get-Item will throw a System.Security.SecurityException with the message "Requested registry access is denied" exception if you try to get the registry key when impersonating with inadequate permissions, which might be a way of sidestepping the Test-Path bug.

I tried this change to Get-TempRegistry in Environment.ps1 instead of using Test-Path and it correctly detected the Access Denied exception when impersonating (although I'm not sure what we should really be doing when that happens - e.g. re-throw or swallow the exception?)...

function Get-TempRegistry {
    $pesterTempRegistryRoot = 'Microsoft.PowerShell.Core\Registry::HKEY_CURRENT_USER\Software\Pester'
    try
    {
        $tempRegistry = Get-Item -Path $pesterTempRegistryRoot -ErrorAction "Stop"
    }
    catch [System.Security.SecurityException]
    { 
        write-host "access denied to registry"
        throw
    }
    catch [System.Management.Automation.ItemNotFoundException]
    {
        write-host "creating tempregistry key"
        $null = New-Item -Path $pesterTempRegistryRoot -ErrorAction Stop
    }
    return $pesterTempRegistryRoot
}

Unfortunately there's a few other bits of TestRegistry.ps1 that expect the "TestRegistry:" ps-drive to exist so they then throw exceptions further down the line - e.g.:

Describing example describe
access denied to registry
  [-] Error occurred in Describe block 0ms
    DriveNotFoundException: Cannot find drive. A drive with the name 'TestRegistry' does not exist.
    at Get-TestRegistryPath, C:\src\scratch\pester\issue-1340\pester\Functions\TestRegistry.ps1: line 60
    at Clear-TestRegistry, C:\src\scratch\pester\issue-1340\pester\Functions\TestRegistry.ps1: line 69
    at DescribeImpl, C:\src\scratch\pester\issue-1340\pester\Functions\Describe.ps1: line 219

so there might be a cascade of changes needed to fix this properly...

mikeclayton avatar Aug 19 '19 21:08 mikeclayton

Has there been any work done on the underlying issue? I don't see any linked issues/PRs but since it's tied to other components, maybe some related work has improved things. Thanks!

rnelson0 avatar Dec 30 '19 13:12 rnelson0

Nope, the test-path code is still there so it is most likely not fixed.

nohwnd avatar Dec 30 '19 15:12 nohwnd

TestRegistry now be disabled, #2008 Creating a PR to throw when access is denied to avoid the hang. Using Test-Path -PathType Container ... throws access denied as expected.

fflaten avatar Aug 10 '22 11:08 fflaten