diff --git a/scripts/Bootstrap/Bootstrap.Integration.Tests.ps1 b/scripts/Bootstrap/Bootstrap.Integration.Tests.ps1 index a8732910..48f85c82 100644 --- a/scripts/Bootstrap/Bootstrap.Integration.Tests.ps1 +++ b/scripts/Bootstrap/Bootstrap.Integration.Tests.ps1 @@ -6,8 +6,9 @@ # What these tests do not cover is swapped out by global aliases, which win over cmdlets and # over functions a script defines: downloads (served from a scratch folder), signatures, the # catalog check, the guard decision, the deployment marker, the agent task, the agent's -# --install and sleeps. Every swap writes a trace line, and the tests check the lines they rely -# on, so a swap that stops working turns a test red instead of reaching the real machine. +# --install (a real program start from PowerShell's location, with the agent's work left out) +# and sleeps. Every swap writes a trace line, and the tests check the lines they rely on, so a +# swap that stops working turns a test red instead of reaching the real machine. # # Run elevated (CI also runs it as SYSTEM), the folder permissions are the real ones. Without # elevation the test account may not make Administrators the owner, so the protected DACL is @@ -114,7 +115,15 @@ function global:HarnessGetScheduledTask { function global:HarnessInvokeAgentInstallMode { param([string]$ExePath, [string[]]$Arguments = @('--install')) Write-HarnessTrace 'Invoke-AgentInstallMode' ('{0} {1} exists={2}' -f $ExePath, ($Arguments -join ' '), [System.IO.File]::Exists($ExePath)) - return [int]$global:HarnessOptions.InstallExitCode + # A real program start from PowerShell's own location, as the agent start is: a location + # that no longer exists fails here with error 267, as it did on the MSI channel. With + # AgentStartFails the stub payload itself is started, a text file that never runs. + if ($global:HarnessOptions.AgentStartFails) { + & $ExePath @Arguments | Out-Host + return $LASTEXITCODE + } + & "$env:SystemRoot\System32\cmd.exe" /c "exit $([int]$global:HarnessOptions.InstallExitCode)" | Out-Host + return $LASTEXITCODE } function global:HarnessGetProcess { @@ -188,6 +197,31 @@ if ($global:HarnessOptions.Iex) { exit $LASTEXITCODE '@ + # A ZIP with exactly these entry names. + function New-ZipWithEntries { + param([string]$Path, [string[]]$Names) + Add-Type -AssemblyName System.IO.Compression + Add-Type -AssemblyName System.IO.Compression.FileSystem + $archive = [System.IO.Compression.ZipFile]::Open($Path, [System.IO.Compression.ZipArchiveMode]::Create) + try { + foreach ($name in $Names) { + $writer = New-Object System.IO.StreamWriter($archive.CreateEntry($name).Open()) + try { $writer.Write("content of $name") } finally { $writer.Dispose() } + } + } finally { $archive.Dispose() } + } + + # Serves a package with one entry that points outside the payload folder, with the + # matching manifest hash, under the stable names. + function Set-EscapingPackage { + param($Scenario) + $zip = Join-Path $Scenario.Server 'AutopilotMonitor-Agent.zip' + Remove-Item -LiteralPath $zip + New-ZipWithEntries -Path $zip -Names @('AutopilotMonitor.Agent.exe', 'AutopilotMonitor.Agent.cat', '../../../outside.txt') + $sha = (Get-FileHash -LiteralPath $zip -Algorithm SHA256).Hash.ToLowerInvariant() + Set-Content -LiteralPath (Join-Path $Scenario.Server 'version.json') -Value ('{"version":"2.0.9999","sha256":"' + $sha + '"}') + } + function New-Scenario { $root = Join-Path $TestDrive ('s' + [guid]::NewGuid().ToString('N').Substring(0, 7)) $scenario = [pscustomobject]@{ @@ -228,6 +262,7 @@ exit $LASTEXITCODE CatalogValid = $true Elevated = $true Iex = $false + AgentStartFails = $false } foreach ($key in $Options.Keys) { $settings[$key] = $Options[$key] } $settings.SwapElevation = ([bool]$settings.Elevated -ne $script:isElevated) @@ -333,7 +368,7 @@ exit $LASTEXITCODE ) } -Describe 'loader 1.3' { +Describe 'loader 1.4' { It 'installs and leaves only the agent folder, with its log carried over' { $s = New-Scenario $run = Invoke-Scenario $s -Script $script:loaderPath -Arguments @('-MaxBootstrapWindowHours', '24') @@ -349,7 +384,7 @@ Describe 'loader 1.3' { @($run.Trace | Where-Object { $_ -like 'Test-FileCatalog|*' }).Count | Should -Be 1 @($run.Trace | Where-Object { $_ -like 'Invoke-AgentInstallMode|*\AutopilotMonitor\Agent\AutopilotMonitor.Agent.exe --install exists=True' }).Count | Should -Be 1 $run.StdOut | Should -Match 'Bootstrap Completed Successfully' - Get-Content -LiteralPath (Join-Path $s.Base 'Logs\bootstrap-loader.log') -Raw | Should -Match '=== Autopilot Monitor bootstrap loader v1\.3 started' + Get-Content -LiteralPath (Join-Path $s.Base 'Logs\bootstrap-loader.log') -Raw | Should -Match '=== Autopilot Monitor bootstrap loader v1\.4 started' if ($script:isElevated) { (Get-Item -LiteralPath $s.Base).GetAccessControl('Owner').GetOwner([System.Security.Principal.SecurityIdentifier]).Value | Should -Be 'S-1-5-32-544' } @@ -400,7 +435,7 @@ Describe 'loader 1.3' { Get-Tree $s.ProgramData -Content | Should -Be $before } - It 'keeps its log when the installer fails, and the next run removes it' { + It 'keeps its log when the installer fails; a later run removes it, a run within the hour leaves it' { $s = New-Scenario Remove-Item -LiteralPath (Join-Path $s.Server 'version.json') $failed = Invoke-Scenario $s -Script $script:loaderPath @@ -418,17 +453,25 @@ Describe 'loader 1.3' { )) Set-Content -LiteralPath (Join-Path $s.Server 'version.json') -Value ('{"version":"2.0.9999","sha256":"' + $script:agentZipSha256 + '"}') - $next = Invoke-Scenario $s -Script $script:loaderPath + $kept = (Get-ChildItem -LiteralPath $s.ProgramData -Directory -Filter 'AutopilotMonitor-Loader-*').FullName - $next.ExitCode | Should -Be 0 - $next.StdErr | Should -BeNullOrEmpty - Get-Tree $s.ProgramData | Should -Be (Join-Tree ($script:installedTree + 'AutopilotMonitor\Logs\bootstrap-loader.log' + 'AutopilotMonitor\Logs\bootstrap-script.log')) + # Minutes later the folder may still belong to a running loader: it stays. + $soon = Invoke-Scenario $s -Script $script:loaderPath + $soon.ExitCode | Should -Be 0 + $soon.StdErr | Should -BeNullOrEmpty + Test-Path -LiteralPath $kept | Should -BeTrue + Get-Tree $s.Base | Should -Be (Join-Tree ((@($script:installedTree) | ForEach-Object { $_ -replace '^AutopilotMonitor\\', '' } | Where-Object { $_ }) + 'Logs\bootstrap-loader.log' + 'Logs\bootstrap-script.log')) + + [System.IO.Directory]::SetCreationTimeUtc($kept, [System.DateTime]::UtcNow.AddHours(-2)) + $later = Invoke-Scenario $s -Script $script:loaderPath -Options @{ Install = $false } + $later.ExitCode | Should -Be 0 + Test-Path -LiteralPath $kept | Should -BeFalse } It 'runs nothing it cannot verify: ' -TestCases @( @{ Case = 'unsigned'; Options = @{ SignatureStatus = 'NotSigned' }; Edit = $null } @{ Case = 'another publisher'; Options = @{ SignerSubject = 'CN=Somebody Else, O=Somebody Else Ltd, C=US' }; Edit = $null } - @{ Case = 'an installer below 2.7'; Options = @{}; Edit = @('$ScriptVersion = "2.7"', '$ScriptVersion = "2.6"') } + @{ Case = 'an installer below 2.8'; Options = @{}; Edit = @('$ScriptVersion = "2.8"', '$ScriptVersion = "2.7"') } @{ Case = 'not the installer'; Options = @{}; Edit = @('$ScriptRole = "AutopilotMonitor-Installer"', '$ScriptRole = "AutopilotMonitor-Other"') } ) { $s = New-Scenario @@ -485,10 +528,13 @@ Describe 'installer assigned directly' { Get-Content -LiteralPath (Join-Path $s.Outside 'target\sentinel.txt') | Should -Be 'must survive' } + # An agent that ran owns its folder, even after a failed --install; one that never started + # leaves only payload, which a later run would take for an agent. It 'fails with exit code 1 and exactly one line on stderr: ' -TestCases @( - @{ Case = 'the download fails'; Missing = 'AutopilotMonitor-Agent.zip'; Options = @{}; Message = 'AutopilotMonitor bootstrap failed: The remote server returned an error: (404) Not Found.*' } - @{ Case = 'the payload fails its catalog'; Missing = $null; Options = @{ CatalogValid = $false }; Message = 'AutopilotMonitor bootstrap failed: Payload verification FAILED*' } - @{ Case = 'the agent install fails'; Missing = $null; Options = @{ InstallExitCode = 7 }; Message = 'AutopilotMonitor bootstrap failed: Agent install failed with exit code 7' } + @{ Case = 'the download fails'; Missing = 'AutopilotMonitor-Agent.zip'; Options = @{}; Message = 'AutopilotMonitor bootstrap failed: The remote server returned an error: (404) Not Found.*'; AgentFolder = 'gone' } + @{ Case = 'the payload fails its catalog'; Missing = $null; Options = @{ CatalogValid = $false }; Message = 'AutopilotMonitor bootstrap failed: Payload verification FAILED*'; AgentFolder = 'gone' } + @{ Case = 'the agent install fails'; Missing = $null; Options = @{ InstallExitCode = 7 }; Message = 'AutopilotMonitor bootstrap failed: Agent install failed with exit code 7'; AgentFolder = 'stays' } + @{ Case = 'the agent does not start'; Missing = $null; Options = @{ AgentStartFails = $true }; Message = "AutopilotMonitor bootstrap failed: Program 'AutopilotMonitor.Agent.exe' failed to run*"; AgentFolder = 'gone' } ) { if ($Missing) { Remove-Item -LiteralPath (Join-Path $s.Server $Missing) } $run = Invoke-Scenario $s -Script $imeCopy -Options $Options @@ -497,9 +543,21 @@ Describe 'installer assigned directly' { @(Get-StdErrLines $run).Count | Should -Be 1 @(Get-StdErrLines $run)[0] | Should -BeLike $Message Get-Tree $s.ProgramData | Should -Not -Match 'Agent-Install' + Test-Path -LiteralPath (Join-Path $s.Base 'Agent') | Should -Be ($AgentFolder -eq 'stays') Test-Path -LiteralPath (Join-Path $s.Base 'Logs\bootstrap-script.log') | Should -BeTrue } + It 'refuses an agent package with an entry outside the payload folder and writes nothing outside' { + Set-EscapingPackage $s + $run = Invoke-Scenario $s -Script $imeCopy + + $run.ExitCode | Should -Be 1 + @(Get-StdErrLines $run).Count | Should -Be 1 + @(Get-StdErrLines $run)[0] | Should -BeLike 'AutopilotMonitor bootstrap failed: The agent package has an entry that would be written outside the payload folder*' + Get-Tree $s.Root | Should -Not -Match 'outside\.txt' + Get-Tree $s.ProgramData | Should -Be (Join-Tree @('AutopilotMonitor\', 'AutopilotMonitor\Logs\', 'AutopilotMonitor\Logs\bootstrap-script.log')) + } + It 'refuses to run without SYSTEM or administrator rights and changes nothing' { Add-File (Join-Path $s.Base 'Logs\bootstrap-script.log') 'earlier run' $before = Get-Tree $s.ProgramData -Content @@ -587,12 +645,13 @@ Describe 'loader 1.2, frozen' { Describe 'bootstrap MSI layouts, started in the folder the MSI installed' { It ' skips and leaves ' -TestCases @( + @{ Name = 'MSI 1.3 (loader in Program Files)'; File = 'Start-AutopilotMonitor.ps1'; Source = 'loader'; Location = 'program-files'; Arguments = @('-LogFileName', 'bootstrap-msi.log'); Left = 'nothing'; Tree = @() } @{ Name = 'MSI 1.2 (loader 1.3)'; File = 'Start-AutopilotMonitor.ps1'; Source = 'loader'; Arguments = @('-LogFileName', 'bootstrap-msi.log'); Left = 'nothing'; Tree = @() } @{ Name = 'MSI 1.1 (loader 1.2)'; File = 'Start-AutopilotMonitor.ps1'; Source = 'loader12'; Arguments = @('-LogFileName', 'bootstrap-msi.log'); Left = 'nothing'; Tree = @() } @{ Name = 'MSI 1.0 (own runner)'; File = 'Invoke-BootstrapDownload.ps1'; Source = 'msiRunner'; Arguments = @(); Left = 'only the log the runner holds open'; Tree = @('AutopilotMonitor\', 'AutopilotMonitor\Logs\', 'AutopilotMonitor\Logs\bootstrap-msi.log') } ) { $s = New-Scenario - $msi = Join-Path $s.Base 'Msi' + $msi = if ($Location -eq 'program-files') { Join-Path $s.Root 'ProgramFiles\AutopilotMonitor\Bootstrap' } else { Join-Path $s.Base 'Msi' } New-Item -ItemType Directory -Path $msi -Force | Out-Null $sourcePath = @{ loader = $script:loaderPath; loader12 = $script:loader12Path; msiRunner = $script:msiRunnerPath }[$Source] Copy-Item -LiteralPath $sourcePath -Destination (Join-Path $msi $File) @@ -604,12 +663,13 @@ Describe 'bootstrap MSI layouts, started in the folder the MSI installed' { } It ' installs' -TestCases @( + @{ Name = 'MSI 1.3 (loader in Program Files)'; File = 'Start-AutopilotMonitor.ps1'; Source = 'loader'; Location = 'program-files'; Arguments = @('-LogFileName', 'bootstrap-msi.log') } @{ Name = 'MSI 1.2 (loader 1.3)'; File = 'Start-AutopilotMonitor.ps1'; Source = 'loader'; Arguments = @('-LogFileName', 'bootstrap-msi.log') } @{ Name = 'MSI 1.1 (loader 1.2)'; File = 'Start-AutopilotMonitor.ps1'; Source = 'loader12'; Arguments = @('-LogFileName', 'bootstrap-msi.log') } @{ Name = 'MSI 1.0 (own runner)'; File = 'Invoke-BootstrapDownload.ps1'; Source = 'msiRunner'; Arguments = @() } ) { $s = New-Scenario - $msi = Join-Path $s.Base 'Msi' + $msi = if ($Location -eq 'program-files') { Join-Path $s.Root 'ProgramFiles\AutopilotMonitor\Bootstrap' } else { Join-Path $s.Base 'Msi' } New-Item -ItemType Directory -Path $msi -Force | Out-Null $sourcePath = @{ loader = $script:loaderPath; loader12 = $script:loader12Path; msiRunner = $script:msiRunnerPath }[$Source] Copy-Item -LiteralPath $sourcePath -Destination (Join-Path $msi $File) @@ -651,6 +711,17 @@ Describe 'OOBE script, as the backend generates it' { @($run.Trace | Where-Object { $_ -like 'Invoke-AgentInstallMode|*' }).Count | Should -Be 1 } + It 'refuses an agent package with an entry outside the payload folder and writes nothing outside' { + $s = New-Scenario + Set-EscapingPackage $s + $run = Invoke-Scenario $s -Script $script:oobeScriptPath -Options @{ Iex = $true } + + $run.ExitCode | Should -Be 1 + $run.StdOut | Should -Match 'The agent package has an entry that would be written outside the payload folder' + Get-Tree $s.Root | Should -Not -Match 'outside\.txt' + Get-Tree $s.ProgramData | Should -Be (Join-Tree @('AutopilotMonitor\', 'AutopilotMonitor\Logs\', 'AutopilotMonitor\Logs\oobe_bootstrap.log')) + } + It 'refuses to run without SYSTEM or administrator rights and creates nothing' { $s = New-Scenario $run = Invoke-Scenario $s -Script $script:oobeScriptPath -Options @{ Iex = $true; Elevated = $false } @@ -697,9 +768,9 @@ Describe 'publish pipeline and dev channel' { } It 'refuses to publish an installer the loader would refuse: ' -TestCases @( - @{ Case = 'below the minimum version'; Old = '$ScriptVersion = "2.7"'; New = '$ScriptVersion = "2.6"' } + @{ Case = 'below the minimum version'; Old = '$ScriptVersion = "2.8"'; New = '$ScriptVersion = "2.7"' } @{ Case = 'without its role line'; Old = '$ScriptRole = "AutopilotMonitor-Installer"'; New = '# no role' } - @{ Case = 'with a second version line'; Old = '$ScriptVersion = "2.7"'; New = "`$ScriptVersion = `"2.7`"`n`$ScriptVersion = `"2.8`"" } + @{ Case = 'with a second version line'; Old = '$ScriptVersion = "2.8"'; New = "`$ScriptVersion = `"2.8`"`n`$ScriptVersion = `"2.9`"" } ) { $root = Join-Path $TestDrive ('repo' + [guid]::NewGuid().ToString('N').Substring(0, 6)) New-Item -ItemType Directory -Path (Join-Path $root 'scripts\Bootstrap') -Force | Out-Null @@ -746,7 +817,7 @@ Describe 'publish pipeline and dev channel' { $run.ExitCode | Should -Be 0 $run.StdErr | Should -BeNullOrEmpty - $run.StdOut | Should -Match 'Installer accepted: AutopilotMonitor-Installer v2\.7' + $run.StdOut | Should -Match 'Installer accepted: AutopilotMonitor-Installer v2\.8' $run.StdOut | Should -Match 'Bootstrap Completed Successfully' } } diff --git a/scripts/Bootstrap/Install-AutopilotMonitor.Tests.ps1 b/scripts/Bootstrap/Install-AutopilotMonitor.Tests.ps1 index 421136c8..c748bcdb 100644 --- a/scripts/Bootstrap/Install-AutopilotMonitor.Tests.ps1 +++ b/scripts/Bootstrap/Install-AutopilotMonitor.Tests.ps1 @@ -53,6 +53,20 @@ BeforeAll { return $p } + # A ZIP with exactly these entry names, for the containment checks. + function New-ZipWithEntries { + param([string]$Path, [string[]]$Names) + Add-Type -AssemblyName System.IO.Compression + Add-Type -AssemblyName System.IO.Compression.FileSystem + $archive = [System.IO.Compression.ZipFile]::Open($Path, [System.IO.Compression.ZipArchiveMode]::Create) + try { + foreach ($name in $Names) { + $writer = New-Object System.IO.StreamWriter($archive.CreateEntry($name).Open()) + try { $writer.Write("content of $name") } finally { $writer.Dispose() } + } + } finally { $archive.Dispose() } + } + function New-CategoryError { param([System.Management.Automation.ErrorCategory]$Category) return New-Object System.Management.Automation.ErrorRecord((New-Object System.Exception 'simulated'), 'Simulated', $Category, $null) @@ -936,6 +950,34 @@ Describe 'Install-VerifiedAgentPayload' { } } +Describe 'Expand-AgentPayload' { + BeforeEach { + $script:zip = Join-Path $TestDrive "package-$([guid]::NewGuid().ToString('N')).zip" + $script:area = New-TestFolder 'area' + $script:destination = Join-Path $script:area 'Agent' + } + + It 'extracts files and subfolders into the destination' { + New-ZipWithEntries -Path $script:zip -Names @('AutopilotMonitor.Agent.exe', 'sub/inner.dll', 'a..b.txt') + Expand-AgentPayload -ZipPath $script:zip -Destination $script:destination + $extracted = @(Get-ChildItem -LiteralPath $script:destination -Recurse -File | ForEach-Object { $_.FullName.Substring($script:destination.Length + 1) } | Sort-Object) + $extracted | Should -Be @(@('AutopilotMonitor.Agent.exe', 'sub\inner.dll', 'a..b.txt') | Sort-Object) + } + + It 'refuses a package with an entry , writing nothing' -TestCases @( + @{ Case = 'above the destination'; Name = '..\outside.txt' } + @{ Case = 'climbing out of a subfolder'; Name = 'sub/../../outside.txt' } + @{ Case = 'with a rooted name'; Name = 'C:\outside.txt' } + @{ Case = 'with a leading separator'; Name = '/outside.txt' } + @{ Case = 'with a drive-relative name'; Name = 'C:outside.txt' } + ) { + New-ZipWithEntries -Path $script:zip -Names @('AutopilotMonitor.Agent.exe', $Name) + { Expand-AgentPayload -ZipPath $script:zip -Destination $script:destination } | Should -Throw '*outside the payload folder*' + Test-Path -LiteralPath $script:destination | Should -BeFalse + @(Get-ChildItem -LiteralPath $script:area -Recurse -Force).Count | Should -Be 0 + } +} + Describe 'Invoke-AgentInstallMode' { It 'returns only the exit code; the program''s output goes to the host' { $r = @(Invoke-AgentInstallMode -ExePath "$env:SystemRoot\System32\cmd.exe" -Arguments @('/c', 'echo from-agent & exit 3')) @@ -955,7 +997,16 @@ Describe 'Read-LoaderLogs' { $env:ProgramData = $script:realProgramData } - It 'takes the whole log of loader 1.3 from the run folder it started this script from' { + It 'takes nothing from a run folder that is not ours' { + Mock Test-IsOwnedByUs { $false } + $run = Join-Path $env:ProgramData 'AutopilotMonitor-Loader-0a1b2c3d' + New-Item -ItemType Directory -Path $run | Out-Null + Set-Content -Path (Join-Path $run 'bootstrap-loader.log') -Value 'planted' + @(Read-LoaderLogs -ScriptPath (Join-Path $run 'Install-AutopilotMonitor.ps1') -ScriptRoot $run).Count | Should -Be 0 + } + + It 'takes the whole log of loader 1.3 or later from the run folder it started this script from' { + Mock Test-IsOwnedByUs { $true } $run = Join-Path $env:ProgramData 'AutopilotMonitor-Loader-0a1b2c3d' New-Item -ItemType Directory -Path $run | Out-Null Set-Content -Path (Join-Path $run 'bootstrap-msi.log') -Value '[2026-10-22 10:00:00] === Autopilot Monitor bootstrap loader v1.3 started (user=SYSTEM, pid=1) ===' @@ -1030,6 +1081,7 @@ Describe 'Remove-StaleLoaderRunFolders' { $stale = Join-Path $env:ProgramData 'AutopilotMonitor-Loader-22222222' New-Item -ItemType Directory -Path $current, $stale | Out-Null Set-Content -Path (Join-Path $stale 'bootstrap-loader.log') -Value 'x' + [System.IO.Directory]::SetCreationTimeUtc($stale, [System.DateTime]::UtcNow.AddHours(-2)) Remove-StaleLoaderRunFolders -ScriptRoot $current @@ -1037,6 +1089,16 @@ Describe 'Remove-StaleLoaderRunFolders' { Test-Path $stale | Should -BeFalse } + It 'keeps a run folder younger than an hour: a loader may still be running there' { + Mock Test-IsOwnedByUs { $true } + $young = Join-Path $env:ProgramData 'AutopilotMonitor-Loader-55555555' + New-Item -ItemType Directory -Path $young | Out-Null + + Remove-StaleLoaderRunFolders -ScriptRoot $null + + Test-Path $young | Should -BeTrue + } + It 'leaves alone what is not ours or is a link' { Mock Test-IsOwnedByUs { $false } $foreign = Join-Path $env:ProgramData 'AutopilotMonitor-Loader-33333333' @@ -1060,13 +1122,19 @@ Describe 'script text' { It 'declares the role and version the loader checks, one line each' { ([regex]::Matches($script:text, '(?m)^\$ScriptRole = "AutopilotMonitor-Installer"\s*$')).Count | Should -Be 1 - ([regex]::Matches($script:text, '(?m)^\$ScriptVersion = "2\.7"\s*$')).Count | Should -Be 1 + ([regex]::Matches($script:text, '(?m)^\$ScriptVersion = "2\.8"\s*$')).Count | Should -Be 1 } It 'never stages anything in the shared temp folder' { $script:text | Should -Not -Match 'env:TEMP' } + # Whether every entry stays inside the payload folder is this script's own check. + It 'never extracts with Expand-Archive' { + $expands = $script:ast.FindAll({ param($n) $n -is [System.Management.Automation.Language.CommandAst] -and $n.GetCommandName() -eq 'Expand-Archive' }, $true) + @($expands).Count | Should -Be 0 + } + It 'never deletes recursively except through Remove-OwnTree' { $removeItems = $script:ast.FindAll({ param($n) $n -is [System.Management.Automation.Language.CommandAst] -and $n.GetCommandName() -in @('Remove-Item', 'ri', 'rm', 'del', 'rd', 'rmdir', 'erase') }, $true) @($removeItems).Count | Should -Be 0 diff --git a/scripts/Bootstrap/Install-AutopilotMonitor.ps1 b/scripts/Bootstrap/Install-AutopilotMonitor.ps1 index 02ca1d1c..0b4a133a 100644 --- a/scripts/Bootstrap/Install-AutopilotMonitor.ps1 +++ b/scripts/Bootstrap/Install-AutopilotMonitor.ps1 @@ -53,7 +53,7 @@ param( ) # The loader checks these two lines before it runs this script. -$ScriptVersion = "2.7" +$ScriptVersion = "2.8" $ScriptRole = "AutopilotMonitor-Installer" # Signer of the agent's file catalog; a subject pattern, so certificate renewals keep working. @@ -517,6 +517,42 @@ function Get-AgentPayloadRejections { return , $rejected } +# Extracts the agent ZIP into the payload folder. Every entry is checked before the first one +# is written: a rooted name, or one whose resolved path lies outside the folder, refuses the +# whole package. The guarantee is this script's own, not the device's Expand-Archive's. +function Expand-AgentPayload { + param([string]$ZipPath, [string]$Destination) + Add-Type -AssemblyName System.IO.Compression + Add-Type -AssemblyName System.IO.Compression.FileSystem + $root = [System.IO.Path]::GetFullPath($Destination).TrimEnd('\') + '\' + $archive = [System.IO.Compression.ZipFile]::OpenRead($ZipPath) + try { + $planned = New-Object System.Collections.Generic.List[object] + foreach ($entry in $archive.Entries) { + $name = $entry.FullName + $target = $null + if (-not [System.IO.Path]::IsPathRooted($name) -and -not $name.Contains(':')) { + $target = [System.IO.Path]::GetFullPath([System.IO.Path]::Combine($root, $name)) + } + if ($null -eq $target -or -not $target.StartsWith($root, [System.StringComparison]::OrdinalIgnoreCase)) { + throw "The agent package has an entry that would be written outside the payload folder ($name); refusing the package." + } + $planned.Add([pscustomobject]@{ Entry = $entry; Target = $target }) + } + [System.IO.Directory]::CreateDirectory($root) | Out-Null + foreach ($item in $planned) { + if ($item.Entry.FullName.EndsWith('/') -or $item.Entry.FullName.EndsWith('\')) { + [System.IO.Directory]::CreateDirectory($item.Target) | Out-Null + continue + } + [System.IO.Directory]::CreateDirectory([System.IO.Path]::GetDirectoryName($item.Target)) | Out-Null + [System.IO.Compression.ZipFileExtensions]::ExtractToFile($item.Entry, $item.Target, $false) + } + } finally { + $archive.Dispose() + } +} + # Verifies the payload in the run folder and moves it in as the agent folder. The move fails # when that folder exists, so nothing is ever merged into it. function Install-VerifiedAgentPayload { @@ -529,7 +565,7 @@ function Install-VerifiedAgentPayload { ) $payload = Join-Path $StagingPath 'Agent' - Expand-Archive -Path $ZipPath -DestinationPath $payload -Force -ErrorAction Stop + Expand-AgentPayload -ZipPath $ZipPath -Destination $payload $rejections = Get-AgentPayloadRejections -Path $payload -ExpectedPublisher $ExpectedPublisher -CatalogName $CatalogName if ($rejections.Count -gt 0) { @@ -587,13 +623,13 @@ function Test-IsLegacyInstallerPath { } # The loader's log, carried into Logs for the agent and the diagnostics package: whole from a -# loader v1.3 run folder; from older loaders, whose log folder users could write to, only the -# start line. Callers wrap the result in @(); never throws. +# loader run folder that is ours; from older loaders, whose log folder users could write to, +# only the start line. Callers wrap the result in @(); never throws. function Read-LoaderLogs { param([string]$ScriptPath, [string]$ScriptRoot) $logs = New-Object System.Collections.Generic.List[object] try { - if (Test-IsLoaderRunFolder $ScriptRoot) { + if ((Test-IsLoaderRunFolder $ScriptRoot) -and (Get-FolderState $ScriptRoot) -eq 'Ours') { foreach ($file in [System.IO.Directory]::GetFiles($ScriptRoot, '*.log')) { $logs.Add([pscustomobject]@{ Name = [System.IO.Path]::GetFileName($file); Text = [System.IO.File]::ReadAllText($file) }) } @@ -628,14 +664,16 @@ function Write-LoaderLogs { } } -# Run folders that failed loader runs kept for diagnosis. Never throws. +# Run folders that failed loader runs kept for diagnosis. A folder younger than an hour may +# belong to a loader that is still running and stays. Never throws. function Remove-StaleLoaderRunFolders { param([string]$ScriptRoot) try { $folders = [System.IO.Directory]::GetDirectories($env:ProgramData, $LoaderRunFolderPrefix + '*') } catch { return } + $cutoff = [System.DateTime]::UtcNow.AddHours(-1) foreach ($folder in $folders) { if ($ScriptRoot -and [string]::Equals([System.IO.Path]::GetFullPath($folder), [System.IO.Path]::GetFullPath($ScriptRoot), [System.StringComparison]::OrdinalIgnoreCase)) { continue } try { - if ((Get-FolderState $folder) -eq 'Ours') { $null = Remove-OwnTree -Path $folder } + if ((Get-FolderState $folder) -eq 'Ours' -and [System.IO.Directory]::GetCreationTimeUtc($folder) -lt $cutoff) { $null = Remove-OwnTree -Path $folder } } catch { } } } @@ -661,9 +699,11 @@ function Invoke-Bootstrap { exit 1 } - # The bootstrap MSI starts PowerShell inside the agent folder, and a working directory - # cannot be deleted. + # The bootstrap MSI starts PowerShell inside the agent folder: a working directory cannot + # be deleted, and PowerShell starts programs from its own location, which must not point + # at a folder that is gone by then. [System.IO.Directory]::SetCurrentDirectory("$env:SystemRoot\System32") + Set-Location -LiteralPath "$env:SystemRoot\System32" Write-Log "===== Autopilot Monitor Bootstrap Started =====" Write-Log "Bootstrap script version: v$ScriptVersion" @@ -788,7 +828,18 @@ function Invoke-Bootstrap { } Write-Log "Calling agent install mode (--install)..." - $installExitCode = Invoke-AgentInstallMode -ExePath $agentExePath + try { + $installExitCode = Invoke-AgentInstallMode -ExePath $agentExePath + } + catch [System.Management.Automation.ApplicationFailedException] { + # The program never started (bad directory, not a program, blocked by policy). An + # agent that ran owns its folder; one that never ran leaves only payload there, which + # every later run would take for an agent. + $left = Remove-OwnTree -Path $AgentBinPath + if ($left.Foreign.Count -eq 0 -and $left.Locked.Count -eq 0) { Write-Log "The agent did not start; $AgentBinPath removed." } + else { Write-Log "The agent did not start; $AgentBinPath not fully removed ($($left.Foreign.Count + $left.Locked.Count) entries stay)." } + throw + } if ($installExitCode -ne 0) { throw "Agent install failed with exit code $installExitCode" } @@ -821,7 +872,8 @@ function Invoke-Bootstrap { Write-Log "Stack trace: $($_.ScriptStackTrace)" if ($LogState.Ready) { Write-Log "Please check log file: $LogFile" } - $errMsg = "AutopilotMonitor bootstrap failed: $($_.Exception.Message)" + # One line: a failed program start carries PowerShell's position lines in its message. + $errMsg = "AutopilotMonitor bootstrap failed: $($_.Exception.Message)" -replace '\s*\r?\n\s*', ' ' if ($errMsg.Length -gt 2048) { $errMsg = $errMsg.Substring(0, 2045) + '...' } [Console]::Error.WriteLine($errMsg) diff --git a/scripts/Bootstrap/Start-AutopilotMonitor.Tests.ps1 b/scripts/Bootstrap/Start-AutopilotMonitor.Tests.ps1 index 299bed87..ebe52199 100644 --- a/scripts/Bootstrap/Start-AutopilotMonitor.Tests.ps1 +++ b/scripts/Bootstrap/Start-AutopilotMonitor.Tests.ps1 @@ -132,7 +132,7 @@ Describe 'Test-InstallerIdentity' { $script:candidate = Join-Path $TestDrive ("installer-" + [guid]::NewGuid().ToString('N') + '.ps1') } - It 'accepts version ' -TestCases @(@{ Version = '2.7' }, @{ Version = '2.10' }, @{ Version = '3.0.1' }) { + It 'accepts version ' -TestCases @(@{ Version = '2.8' }, @{ Version = '2.10' }, @{ Version = '3.0.1' }) { [System.IO.File]::WriteAllText($script:candidate, "`$ScriptVersion = `"$Version`"`n`$ScriptRole = `"AutopilotMonitor-Installer`"`n") Test-InstallerIdentity -Path $script:candidate | Should -BeTrue } @@ -140,14 +140,14 @@ Describe 'Test-InstallerIdentity' { # Every installer the publisher ever signed keeps a valid signature; the minimum keeps the # older ones from running. It 'refuses an installer below the minimum version' { - [System.IO.File]::WriteAllText($script:candidate, "`$ScriptVersion = `"2.6`"`n`$ScriptRole = `"AutopilotMonitor-Installer`"`n") + [System.IO.File]::WriteAllText($script:candidate, "`$ScriptVersion = `"2.7`"`n`$ScriptRole = `"AutopilotMonitor-Installer`"`n") Test-InstallerIdentity -Path $script:candidate | Should -BeFalse - Should -Invoke Write-Log -ParameterFilter { $Message -like '*below the minimum 2.7*' } + Should -Invoke Write-Log -ParameterFilter { $Message -like '*below the minimum 2.8*' } } # The publisher signs other scripts too. It 'refuses another role' { - [System.IO.File]::WriteAllText($script:candidate, "`$ScriptVersion = `"2.7`"`n`$ScriptRole = `"AutopilotMonitor-Uninstaller`"`n") + [System.IO.File]::WriteAllText($script:candidate, "`$ScriptVersion = `"2.8`"`n`$ScriptRole = `"AutopilotMonitor-Uninstaller`"`n") Test-InstallerIdentity -Path $script:candidate | Should -BeFalse } @@ -165,19 +165,28 @@ Describe 'Get-VerifiedBootstrapScript' { BeforeEach { Mock Write-Log { } Mock Start-Sleep { } - Mock Invoke-WebRequest { } + Mock Invoke-WebRequest { [System.IO.File]::WriteAllText($OutFile, 'downloaded') } Mock Test-InstallerIdentity { $true } + # The test account owns the folder; the loader asks for SYSTEM or Administrators. + Mock Test-IsOwnedByUs { $true } + $script:run = Join-Path $TestDrive ('run-' + [guid]::NewGuid().ToString('N').Substring(0, 8)) + New-Item -ItemType Directory -Path $script:run | Out-Null + $script:dest = Join-Path $script:run 'Install-AutopilotMonitor.ps1' + $RunState = [pscustomobject]@{ LogFile = $null; Installer = $null } + } + AfterEach { + if ($RunState.Installer) { $RunState.Installer.Dispose() } } It 'returns after the first attempt when the download verifies' { Mock Test-BootstrapSignature { $true } - Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination 'TestDrive:\s.ps1' | Should -BeTrue + Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination $script:dest -RunFolder $script:run | Should -BeTrue Should -Invoke Invoke-WebRequest -Times 1 -Exactly } It 'gives up after three attempts when verification keeps failing' { Mock Test-BootstrapSignature { $false } - Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination 'TestDrive:\s.ps1' | Should -BeFalse + Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination $script:dest -RunFolder $script:run | Should -BeFalse Should -Invoke Invoke-WebRequest -Times 3 -Exactly } @@ -186,31 +195,55 @@ Describe 'Get-VerifiedBootstrapScript' { It 'retries a failed verification and accepts a later attempt' { $script:calls = 0 Mock Test-BootstrapSignature { $script:calls++; return ($script:calls -ge 2) } - Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination 'TestDrive:\s.ps1' | Should -BeTrue + Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination $script:dest -RunFolder $script:run | Should -BeTrue Should -Invoke Invoke-WebRequest -Times 2 -Exactly } It 'retries a transport failure' { $script:downloads = 0 - Mock Invoke-WebRequest { $script:downloads++; if ($script:downloads -eq 1) { throw 'connection reset' } } + Mock Invoke-WebRequest { $script:downloads++; if ($script:downloads -eq 1) { throw 'connection reset' }; [System.IO.File]::WriteAllText($OutFile, 'downloaded') } Mock Test-BootstrapSignature { $true } - Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination 'TestDrive:\s.ps1' | Should -BeTrue + Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination $script:dest -RunFolder $script:run | Should -BeTrue Should -Invoke Invoke-WebRequest -Times 2 -Exactly } It 'refuses a validly signed script that is not the current installer' { Mock Test-BootstrapSignature { $true } Mock Test-InstallerIdentity { $false } - Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination 'TestDrive:\s.ps1' | Should -BeFalse + Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination $script:dest -RunFolder $script:run | Should -BeFalse Should -Invoke Invoke-WebRequest -Times 3 -Exactly } # The identity lines count only once the signature proves they are the publisher's. It 'reads the identity only after the signature passed' { Mock Test-BootstrapSignature { $false } - Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination 'TestDrive:\s.ps1' | Should -BeFalse + Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination $script:dest -RunFolder $script:run | Should -BeFalse Should -Invoke Test-InstallerIdentity -Times 0 -Exactly } + + It 'stops without downloading when the run folder is no longer a folder of ours' { + Mock Test-IsOwnedByUs { $false } + Mock Test-BootstrapSignature { $true } + Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination $script:dest -RunFolder $script:run | Should -BeFalse + Should -Invoke Invoke-WebRequest -Times 0 -Exactly + } + + # The file that was checked is the file that runs: nobody can replace it in between. + It 'keeps the verified file open, so it cannot be replaced or deleted' { + Mock Test-BootstrapSignature { $true } + Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination $script:dest -RunFolder $script:run | Should -BeTrue + $RunState.Installer | Should -Not -BeNullOrEmpty + { [System.IO.File]::WriteAllText($script:dest, 'replaced') } | Should -Throw + { [System.IO.File]::Delete($script:dest) } | Should -Throw + [System.IO.File]::ReadAllText($script:dest) | Should -Be 'downloaded' + } + + It 'holds nothing when no attempt verifies' { + Mock Test-BootstrapSignature { $false } + Get-VerifiedBootstrapScript -Url 'https://example.invalid/s.ps1' -Destination $script:dest -RunFolder $script:run | Should -BeFalse + $RunState.Installer | Should -BeNullOrEmpty + { [System.IO.File]::Delete($script:dest) } | Should -Not -Throw + } } Describe 'Get-DownloadSecurityProtocol' { @@ -232,13 +265,20 @@ Describe 'Remove-LoaderRunFiles' { $log = Join-Path $run 'bootstrap-loader.log' Set-Content -Path $installer -Value 'downloaded' Set-Content -Path $log -Value 'log' - $LogState = [pscustomobject]@{ File = $log } + $RunState = [pscustomobject]@{ LogFile = $log; Installer = $null } } It 'removes the download, the log and the folder' { Remove-LoaderRunFiles -RunFolder $run -InstallerPath $installer Test-Path -LiteralPath $run | Should -BeFalse - $LogState.File | Should -BeNullOrEmpty + $RunState.LogFile | Should -BeNullOrEmpty + } + + It 'releases the held download before deleting it' { + $RunState.Installer = New-Object System.IO.FileStream($installer, [System.IO.FileMode]::Open, [System.IO.FileAccess]::Read, [System.IO.FileShare]::Read) + Remove-LoaderRunFiles -RunFolder $run -InstallerPath $installer + Test-Path -LiteralPath $run | Should -BeFalse + $RunState.Installer | Should -BeNullOrEmpty } It 'keeps the log and the folder with -KeepLog, but never the download' { @@ -262,7 +302,7 @@ Describe 'Start-BootstrapLoader' { $script:runFolder = Join-Path $env:ProgramData 'AutopilotMonitor-Loader-0a1b2c3d' $script:marker = Join-Path $TestDrive ("ran-" + [guid]::NewGuid().ToString('N') + '.txt') - $LogState = [pscustomobject]@{ File = $null } + $RunState = [pscustomobject]@{ LogFile = $null; Installer = $null } $InstallerArguments = @() Mock Write-Host { } @@ -426,4 +466,12 @@ Describe 'publish contract' { $wxs.Contains('[INSTALLFOLDER]Start-AutopilotMonitor.ps1') | Should -BeTrue $wxs.Contains('-LogFileName bootstrap-msi.log') | Should -BeTrue } + + # The file a SYSTEM custom action runs must not sit where a standard user could have + # created the folder first: Program Files, never ProgramData. + It 'is installed by the bootstrap MSI under Program Files' { + $wxs = Get-Content (Join-Path $PSScriptRoot '..\..\src\Agent\AutopilotMonitor.BootstrapMsi\Package.wxs') -Raw + $wxs | Should -Match '' + $wxs | Should -Not -Match 'CommonAppDataFolder' + } } diff --git a/scripts/Bootstrap/Start-AutopilotMonitor.ps1 b/scripts/Bootstrap/Start-AutopilotMonitor.ps1 index 7be524c0..a9d77156 100644 --- a/scripts/Bootstrap/Start-AutopilotMonitor.ps1 +++ b/scripts/Bootstrap/Start-AutopilotMonitor.ps1 @@ -10,8 +10,10 @@ the installer changes. The download and the log go into %ProgramData%\AutopilotMonitor-Loader-, which only - SYSTEM and administrators can change. After a successful run the folder is removed; after - a failure it keeps the log for diagnosis, and the next installer run removes it. + SYSTEM and administrators can change. The loader keeps the downloaded file open from the + check to the end of the run, so what it checked is what runs. After a successful run the + folder is removed; after a failure it keeps the log for diagnosis, and a later installer + run removes it. The exit code is always 0, so the loader never fails enrollment. Without SYSTEM or administrator rights it stops with an error and changes nothing. @@ -40,28 +42,29 @@ param( # attribute would reject them instead. $InstallerArguments = $args -$ScriptVersion = "1.3" +$ScriptVersion = "1.4" # A subject pattern, not a thumbprint, so a certificate renewal keeps deployed loaders working. $ExpectedPublisher = "*O=glueckkanja AG*" $ExpectedInstallerRole = "AutopilotMonitor-Installer" -$MinimumInstallerVersion = [version]"2.7" +$MinimumInstallerVersion = [version]"2.8" $RunFolderPrefix = "AutopilotMonitor-Loader-" $DownloadAttempts = 3 $RetryDelaySeconds = 10 -$LogState = [pscustomobject]@{ File = $null } +# The log file, and the handle that keeps the verified installer unchanged until the run ends. +$RunState = [pscustomobject]@{ LogFile = $null; Installer = $null } function Write-Log { param([string]$Message) $timestamp = Get-Date -Format "yyyy-MM-dd HH:mm:ss" $logMessage = "[$timestamp] $Message" Write-Host $logMessage - if ($LogState.File) { - Add-Content -Path $LogState.File -Value $logMessage -ErrorAction SilentlyContinue + if ($RunState.LogFile) { + Add-Content -Path $RunState.LogFile -Value $logMessage -ErrorAction SilentlyContinue } } @@ -200,18 +203,28 @@ function Test-BootstrapSignature { return $true } -# A download that fails verification, such as a truncated one, is retried too. +# A download that fails verification, such as a truncated one, is retried too. The run folder +# is checked before every attempt: one that is no longer a real folder of ours stops the run. +# The downloaded file is opened for reading before it is checked and stays open until the run +# ends, so no one can change or replace what was checked. function Get-VerifiedBootstrapScript { - param([string]$Url, [string]$Destination) + param([string]$Url, [string]$Destination, [string]$RunFolder) for ($attempt = 1; $attempt -le $DownloadAttempts; $attempt++) { + if ((Test-IsReparsePoint $RunFolder) -or -not (Test-IsOwnedByUs $RunFolder)) { + Write-Log "REJECTED: the run folder $RunFolder is no longer a folder of ours. Nothing was downloaded." + return $false + } try { Write-Log "Download attempt $attempt/$DownloadAttempts from $Url" Invoke-WebRequest -Uri $Url -OutFile $Destination -UseBasicParsing -TimeoutSec 60 -ErrorAction Stop + $held = New-Object System.IO.FileStream($Destination, [System.IO.FileMode]::Open, [System.IO.FileAccess]::Read, [System.IO.FileShare]::Read) if ((Test-BootstrapSignature -Path $Destination) -and (Test-InstallerIdentity -Path $Destination)) { + $RunState.Installer = $held return $true } + $held.Dispose() } catch { Write-Log "Download attempt $attempt failed: $($_.Exception.Message)" @@ -229,11 +242,15 @@ function Get-VerifiedBootstrapScript { # otherwise they stay for diagnosis. function Remove-LoaderRunFiles { param([string]$RunFolder, [string]$InstallerPath, [switch]$KeepLog) + if ($RunState.Installer) { + $RunState.Installer.Dispose() + $RunState.Installer = $null + } try { if ([System.IO.File]::Exists($InstallerPath)) { [System.IO.File]::Delete($InstallerPath) } if ($KeepLog) { return } - $logFile = $LogState.File - $LogState.File = $null + $logFile = $RunState.LogFile + $RunState.LogFile = $null if ($logFile -and [System.IO.File]::Exists($logFile)) { [System.IO.File]::Delete($logFile) } [System.IO.Directory]::Delete($RunFolder, $false) } @@ -265,14 +282,14 @@ function Start-BootstrapLoader { return } - $LogState.File = Join-Path $runFolder $LogFileName + $RunState.LogFile = Join-Path $runFolder $LogFileName Write-Log "=== Autopilot Monitor bootstrap loader v$ScriptVersion started (user=$env:USERNAME, pid=$PID) ===" Write-Log ("PowerShell {0}, {1}, language mode {2}, run folder {3}" -f $PSVersionTable.PSVersion, $(if ([Environment]::Is64BitProcess) { '64-bit' } else { '32-bit' }), $ExecutionContext.SessionState.LanguageMode, $runFolder) $installerPath = Join-Path $runFolder 'Install-AutopilotMonitor.ps1' $succeeded = $false try { - if (-not (Get-VerifiedBootstrapScript -Url $BootstrapUrl -Destination $installerPath)) { + if (-not (Get-VerifiedBootstrapScript -Url $BootstrapUrl -Destination $installerPath -RunFolder $runFolder)) { Write-Log "No verified installer available after $DownloadAttempts attempt(s). Nothing was executed." return } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Harness/ElevatedFactAttribute.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Harness/ElevatedFactAttribute.cs new file mode 100644 index 00000000..c73879fd --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Harness/ElevatedFactAttribute.cs @@ -0,0 +1,21 @@ +using System.Security.Principal; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Harness +{ + /// + /// A fact that needs an elevated process, such as making the Administrators group the owner + /// of a folder. Skipped in a non-elevated local run; the CI runners are elevated. + /// + public sealed class ElevatedFactAttribute : FactAttribute + { + public ElevatedFactAttribute() + { + using (var identity = WindowsIdentity.GetCurrent()) + { + if (!new WindowsPrincipal(identity).IsInRole(WindowsBuiltInRole.Administrator)) + Skip = "needs an elevated process (assigning Administrators as the folder owner)"; + } + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/AgentDirectoryAclTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/AgentDirectoryAclTests.cs index 3dd03ae6..a6f82fbf 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/AgentDirectoryAclTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/AgentDirectoryAclTests.cs @@ -4,14 +4,16 @@ using System.Linq; using System.Security.AccessControl; using System.Security.Principal; using AutopilotMonitor.Agent.V2.Core.Security; +using AutopilotMonitor.Agent.V2.Core.Tests.Harness; using Xunit; namespace AutopilotMonitor.Agent.V2.Core.Tests.Security { /// - /// The agent data directory keeps standard users from adding files where the SYSTEM agent - /// starts from. Works on a temp folder the test owns; as owner it can always hand itself the - /// rights back to clean up. + /// The agent folders keep standard users from adding files where the SYSTEM agent starts from. + /// Works on a temp folder the test owns; as owner, or as an elevated administrator, it can + /// always hand itself the rights back to clean up. Tests that assign the Administrators group + /// as owner need an elevated process and skip without one. /// public sealed class AgentDirectoryAclTests : IDisposable { @@ -21,6 +23,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security | FileSystemRights.ChangePermissions | FileSystemRights.TakeOwnership; private static readonly SecurityIdentifier Users = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + private static readonly SecurityIdentifier Administrators = new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null); private readonly string _root = Path.Combine(Path.GetTempPath(), "apm-acl-" + Guid.NewGuid().ToString("N")); @@ -51,8 +54,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security .Where(r => r.AccessControlType == AccessControlType.Allow && Users.Equals(r.IdentityReference)) .Aggregate((FileSystemRights)0, (all, r) => all | r.FileSystemRights); + private static SecurityIdentifier OwnerOf(string path) => + (SecurityIdentifier)new DirectoryInfo(path).GetAccessControl(AccessControlSections.Owner).GetOwner(typeof(SecurityIdentifier)); + [Fact] - public void The_protected_security_inherits_nothing_and_gives_users_read_only() + public void The_protected_security_inherits_nothing_gives_users_read_only_and_names_administrators_as_owner() { var security = AgentDirectoryAcl.BuildProtectedSecurity(); @@ -62,9 +68,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security // An allow rule always carries Synchronize; .NET adds it on construction. Assert.Equal(FileSystemRights.ReadAndExecute, users.FileSystemRights & ~FileSystemRights.Synchronize); Assert.Equal((FileSystemRights)0, users.FileSystemRights & WriteRights); + Assert.Equal(Administrators, security.GetOwner(typeof(SecurityIdentifier))); } - [Fact] + [ElevatedFact] public void A_folder_is_protected_once_and_then_left_alone() { var first = AgentDirectoryAcl.EnsureProtected(_root); @@ -72,11 +79,29 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security Assert.Equal(AgentDirectoryAclOutcome.Applied, first.Outcome); Assert.Equal(AgentDirectoryAclOutcome.AlreadyProtected, second.Outcome); - Assert.True(AgentDirectoryAcl.IsProtectedAsExpected(new DirectoryInfo(_root).GetAccessControl(AccessControlSections.Access))); + Assert.True(AgentDirectoryAcl.IsProtectedAsExpected(new DirectoryInfo(_root).GetAccessControl(AccessControlSections.Access | AccessControlSections.Owner))); Assert.Equal((FileSystemRights)0, UsersRights(_root) & WriteRights); + Assert.Equal(Administrators, OwnerOf(_root)); } - [Fact] + // The owner can always hand itself the rights back, so a folder another account created + // first is theirs until the owner changes, whatever its DACL says. + [ElevatedFact] + public void A_folder_owned_by_another_account_gets_administrators_as_owner() + { + var directory = new DirectoryInfo(_root); + var mine = directory.GetAccessControl(AccessControlSections.Owner); + mine.SetOwner(WindowsIdentity.GetCurrent().User!); + directory.SetAccessControl(mine); + Assert.NotEqual(Administrators, OwnerOf(_root)); + + var result = AgentDirectoryAcl.EnsureProtected(_root); + + Assert.Equal(AgentDirectoryAclOutcome.Applied, result.Outcome); + Assert.Equal(Administrators, OwnerOf(_root)); + } + + [ElevatedFact] public void Existing_subfolders_take_over_the_protection() { // Install mode protects the base after Agent, Spool and Logs already exist. @@ -101,6 +126,35 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Security Assert.False(AgentDirectoryAcl.IsProtectedAsExpected(widened)); } + [Fact] + public void A_dacl_under_another_owner_is_not_taken_as_protected() + { + var mine = AgentDirectoryAcl.BuildProtectedSecurity(); + mine.SetOwner(WindowsIdentity.GetCurrent().User!); + + Assert.False(AgentDirectoryAcl.IsProtectedAsExpected(mine)); + } + + [Fact] + public void A_junction_is_refused_and_its_target_left_alone() + { + var target = Directory.CreateDirectory(Path.Combine(_root, "target")).FullName; + var link = Path.Combine(_root, "link"); + NtfsLinks.CreateJunction(link, target); + try + { + var result = AgentDirectoryAcl.EnsureProtected(link); + + Assert.Equal(AgentDirectoryAclOutcome.Failed, result.Outcome); + Assert.Contains("junction", result.Error); + Assert.False(new DirectoryInfo(target).GetAccessControl().AreAccessRulesProtected); + } + finally + { + NtfsLinks.RemoveLink(link); + } + } + [Fact] public void A_missing_folder_is_reported_not_created() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/AgentDirectoryAcl.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/AgentDirectoryAcl.cs index 69c072f3..723886ed 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/AgentDirectoryAcl.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/AgentDirectoryAcl.cs @@ -8,18 +8,20 @@ using System.Security.Principal; namespace AutopilotMonitor.Agent.V2.Core.Security { /// - /// Keeps %ProgramData%\AutopilotMonitor out of reach of standard users. Left alone the - /// folder inherits ProgramData's "Users may create files and folders" entry, and the SYSTEM - /// agent and its boot task start from the Agent subfolder: a user who can add a file there - /// can plant code the next SYSTEM start loads. During the device phase of OOBE only SYSTEM is - /// around, but the agent keeps running through the user phase and on the desktop until the - /// enrollment ends, restarted by the boot task after every reboot; that is the window this - /// closes. The payload catalog does not cover it: it is checked at install time, a file - /// added later is never looked at. + /// Keeps %ProgramData%\AutopilotMonitor and the Agent folder the SYSTEM task runs from + /// out of reach of standard users. Left alone the folder inherits ProgramData's "Users may + /// create files and folders" entry: a user who can add a file where the agent starts can plant + /// code the next SYSTEM start loads. During the device phase of OOBE only SYSTEM is around, but + /// the agent keeps running through the user phase and on the desktop until the enrollment + /// ends, restarted by the boot task after every reboot; that is the window this closes. The + /// payload catalog does not cover it: it is checked at install time, a file added later is + /// never looked at. /// /// The DACL is protected (nothing inherited from ProgramData): SYSTEM and Administrators full - /// control, Users read and execute, all inherited by the subfolders. Explicit entries on a - /// child stay. The summary dialog's temp folder lives outside this tree anyway. + /// control, Users read and execute, all inherited by the subfolders. The owner is Administrators: + /// the owner of a folder can always hand itself the rights back, so a folder a standard user + /// created first stays theirs until the owner changes, whatever the DACL says. A junction or + /// symbolic link is never protected or followed. /// /// public static class AgentDirectoryAcl @@ -31,8 +33,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Security private static readonly SecurityIdentifier Users = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); /// - /// Applies the protected DACL to unless it is already in place. - /// Never throws: a folder the agent cannot protect must not stop the agent. + /// Applies the protected DACL and the owner to unless both are already + /// in place. A reparse point is refused, not followed. Never throws: a folder the agent cannot + /// protect must not crash the agent; the caller decides what the failure means. /// public static AgentDirectoryAclResult EnsureProtected(string path) { @@ -41,8 +44,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Security var directory = new DirectoryInfo(path); if (!directory.Exists) return new AgentDirectoryAclResult(AgentDirectoryAclOutcome.Missing, null); + if ((directory.Attributes & FileAttributes.ReparsePoint) != 0) + return new AgentDirectoryAclResult(AgentDirectoryAclOutcome.Failed, $"{path} is a junction or symbolic link, not a folder"); - if (IsProtectedAsExpected(directory.GetAccessControl(AccessControlSections.Access))) + if (IsProtectedAsExpected(directory.GetAccessControl(AccessControlSections.Access | AccessControlSections.Owner))) return new AgentDirectoryAclResult(AgentDirectoryAclOutcome.AlreadyProtected, null); directory.SetAccessControl(BuildProtectedSecurity()); @@ -54,6 +59,12 @@ namespace AutopilotMonitor.Agent.V2.Core.Security } } + /// + /// Creates the folder with the protected DACL and owner in one step, so there is no moment + /// in which it carries ProgramData's inherited entries. + /// + public static void CreateProtected(string path) => Directory.CreateDirectory(path, BuildProtectedSecurity()); + internal static DirectorySecurity BuildProtectedSecurity() { var security = new DirectorySecurity(); @@ -61,15 +72,21 @@ namespace AutopilotMonitor.Agent.V2.Core.Security security.AddAccessRule(new FileSystemAccessRule(LocalSystem, FileSystemRights.FullControl, Inherit, PropagationFlags.None, AccessControlType.Allow)); security.AddAccessRule(new FileSystemAccessRule(Administrators, FileSystemRights.FullControl, Inherit, PropagationFlags.None, AccessControlType.Allow)); security.AddAccessRule(new FileSystemAccessRule(Users, FileSystemRights.ReadAndExecute, Inherit, PropagationFlags.None, AccessControlType.Allow)); + security.SetOwner(Administrators); return security; } /// - /// True when the DACL is protected and holds exactly the three entries above. Windows adds - /// when it stores an entry, so that bit is ignored. + /// True when the owner is SYSTEM or Administrators and the DACL is protected and holds exactly + /// the three entries above. Windows adds when it + /// stores an entry, so that bit is ignored. /// internal static bool IsProtectedAsExpected(DirectorySecurity security) { + if (!(security.GetOwner(typeof(SecurityIdentifier)) is SecurityIdentifier owner) + || !(owner.IsWellKnown(WellKnownSidType.LocalSystemSid) || owner.IsWellKnown(WellKnownSidType.BuiltinAdministratorsSid))) + return false; + if (!security.AreAccessRulesProtected) return false; diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Program.InstallMode.cs b/src/Agent/AutopilotMonitor.Agent.V2/Program.InstallMode.cs index 4d4686f4..72582eb3 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Program.InstallMode.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Program.InstallMode.cs @@ -57,8 +57,11 @@ namespace AutopilotMonitor.Agent.V2 logger.Info("======================= Agent install mode (--install) ======================="); EnsureAgentDirectories(logger); - LogAgentDirectoryAcl(AgentDirectoryAcl.EnsureProtected( - Environment.ExpandEnvironmentVariables(Constants.AgentDataDirectory)), logger, alwaysLog: true); + if (!ProtectAgentDirectories(logger, alwaysLog: true)) + { + logger.Error("Install refused: the agent folder could not be protected; nothing was installed."); + return 1; + } var sourceExePath = Assembly.GetExecutingAssembly().Location; var sourceDir = Path.GetDirectoryName(sourceExePath) ?? string.Empty; @@ -276,31 +279,48 @@ namespace AutopilotMonitor.Agent.V2 } /// - /// Logs the outcome of . Install mode logs - /// every outcome; the runtime start only a change or a failure, so a normal start stays quiet. + /// Protects the base folder and the Agent folder the task runs from. False when one of them + /// could not be protected: install mode then refuses to go on, a running agent logs it. + /// Install mode logs every outcome; the runtime start only a change or a failure, so a + /// normal start stays quiet. /// - internal static void LogAgentDirectoryAcl(AgentDirectoryAclResult result, AgentLogger logger, bool alwaysLog) + internal static bool ProtectAgentDirectories(AgentLogger logger, bool alwaysLog) { - switch (result.Outcome) + var basePath = Environment.ExpandEnvironmentVariables(Constants.AgentDataDirectory); + var ok = true; + foreach (var path in new[] { basePath, Path.Combine(basePath, DefaultAgentSubdirectory) }) { - case AgentDirectoryAclOutcome.Applied: - logger.Info("Agent data directory protected: SYSTEM and Administrators full control, Users read and execute, nothing inherited from ProgramData."); - break; - case AgentDirectoryAclOutcome.Failed: - logger.Warning($"Could not protect the agent data directory: {result.Error}"); - break; - default: - if (alwaysLog) logger.Info($"Agent data directory ACL: {result.Outcome}."); - break; + var result = AgentDirectoryAcl.EnsureProtected(path); + switch (result.Outcome) + { + case AgentDirectoryAclOutcome.Applied: + logger.Info($"Agent folder {path} protected: owner Administrators; SYSTEM and Administrators full control, Users read and execute, nothing inherited."); + break; + case AgentDirectoryAclOutcome.Failed: + logger.Warning($"Could not protect the agent folder {path}: {result.Error}"); + ok = false; + break; + default: + if (alwaysLog) logger.Info($"Agent folder {path} ACL: {result.Outcome}."); + break; + } } + return ok; } private static void EnsureAgentDirectories(AgentLogger logger) { var basePath = Environment.ExpandEnvironmentVariables(Constants.AgentDataDirectory); + if (!Directory.Exists(basePath)) + { + // Protected from the first moment: created with ProgramData's default entries, the + // folder would let any user add to it until the protection arrives. + AgentDirectoryAcl.CreateProtected(basePath); + logger.Info($"Created directory: {basePath}"); + } + var paths = new[] { - basePath, Path.Combine(basePath, DefaultAgentSubdirectory), Environment.ExpandEnvironmentVariables(Constants.SpoolDirectory), Environment.ExpandEnvironmentVariables(Constants.LogDirectory), diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Program.cs b/src/Agent/AutopilotMonitor.Agent.V2/Program.cs index 07e0ceb1..82552a0a 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Program.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Program.cs @@ -170,8 +170,7 @@ namespace AutopilotMonitor.Agent.V2 // Same protection install mode applies, re-checked on every start: it also reaches // agents that arrived by self-update over an install made before the hardening. - LogAgentDirectoryAcl(AgentDirectoryAcl.EnsureProtected( - Environment.ExpandEnvironmentVariables(Constants.AgentDataDirectory)), logger, alwaysLog: false); + ProtectAgentDirectories(logger, alwaysLog: false); try { diff --git a/src/Agent/AutopilotMonitor.BootstrapMsi/Package.wxs b/src/Agent/AutopilotMonitor.BootstrapMsi/Package.wxs index 2da6ee87..03e8dd4d 100644 --- a/src/Agent/AutopilotMonitor.BootstrapMsi/Package.wxs +++ b/src/Agent/AutopilotMonitor.BootstrapMsi/Package.wxs @@ -11,12 +11,16 @@ logic stays updatable without re-shipping the MSI, and both delivery channels share one implementation. + The loader is installed under Program Files, where only administrators and SYSTEM can + create or change anything: the file that a SYSTEM custom action runs must not live in a + folder a standard user could create first (ProgramData lets every user create folders). + Build: wix build Package.wxs -arch x64 -o bin\AutopilotMonitor-Bootstrap.msi --> @@ -24,16 +28,16 @@ - + - + - + file. New component GUID because the key path changed (1.3.0: Program Files). --> +