diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 55ace603..fef74826 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -172,6 +172,7 @@ jobs: .\deploy\Test-DeploymentTemplates.ps1 .\deploy\Test-ArtifactSecurity.ps1 .\deploy\Test-SetupAdapter.ps1 + .\deploy\Test-LocalServerHttp.ps1 .\deploy\Test-FailoverRecovery.ps1 .\deploy\Test-MachinePath.ps1 .\deploy\Test-SwitcherConfig.ps1 @@ -198,6 +199,7 @@ jobs: run: | .\deploy\Test-ArtifactSecurity.ps1 .\deploy\Test-SetupAdapter.ps1 + .\deploy\Test-LocalServerHttp.ps1 .\deploy\Test-FailoverRecovery.ps1 .\deploy\Test-MachinePath.ps1 .\deploy\Test-SwitcherConfig.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a760ccc..ff9d8719 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,18 @@ Planned version: **1.5.4**. - The MCP server rejects broken nodes and edges before it merges a workflow change (#726). - All MCP requests, including dependency queries, wait until the server connection is ready (#727). +- **Setup and uninstall** + - A server uninstall whose cleanup fails stops before the "Apps & features" entry is removed, + keeps the installation marker and the deploy scripts and exits with a non-zero code, so the + same uninstall can be run again. A failed service deletion stops it before any file is removed + (#714). + - Deleting the data on uninstall (`-PurgeData`, `/PURGEDATA=1`) also works on German Windows. If + ownership of an item cannot be taken, nothing under the data directory is deleted (#731). + - When a reinstall that changes the service account fails, the rollback gives the previous + account its access to the install directory back, so the restored service starts (#735). + - The gMSA repair command suggested by the setup check adds this host to the existing principals + instead of replacing them. The deploy README no longer suggests delegation on every target + (#713). - **Monitoring** - The observability summary queries the counter names the exporter actually publishes, so executions per minute, success rate and failures show values again. The Grafana CPU panels use @@ -121,6 +133,9 @@ Planned version: **1.5.4**. - The CLI, the MCP server and the Python admin scripts under `scripts/` no longer follow redirects with the login token, so the token is never sent to another host (#711). - Logging out with the CLI also ends the MCP server's session right away (#712). +- The server setup accepts only the configured certificate, by thumbprint, for its health probe and + the first login, does not follow redirects there and no longer turns off certificate validation + for the whole process. - MCP alerting output shows webhook targets only as scheme, host and port, because the rest of the URL can contain tokens (#730). - GitHub tokens in step output are masked except for their prefix such as `ghp_`; before, the whole diff --git a/deploy/ArtifactSecurity.ps1 b/deploy/ArtifactSecurity.ps1 index 6ad35e5e..26433606 100644 --- a/deploy/ArtifactSecurity.ps1 +++ b/deploy/ArtifactSecurity.ps1 @@ -697,6 +697,35 @@ function Resolve-SidLabel { } } +function Get-NodePilotDirectoryAccessSnapshot { + <# + The DACL of an existing directory, including its protection flag, so a rollback can give a + previous service identity back exactly the access it had. $null when the directory is absent. + #> + param([Parameter(Mandatory)][string]$Path) + if (-not (Test-Path -LiteralPath $Path -PathType Container)) { return $null } + $access = (Get-Acl -LiteralPath $Path).GetSecurityDescriptorSddlForm( + [System.Security.AccessControl.AccessControlSections]::Access) + return [pscustomobject]@{ Path = $Path; Access = $access } +} + +function Restore-NodePilotDirectoryAccessSnapshot { + <# + Writes a DACL captured by Get-NodePilotDirectoryAccessSnapshot back and verifies it. Inheriting + children pick it up, so it runs before restored binaries are copied in and started. + #> + param([Parameter(Mandatory)][object]$Snapshot) + $acl = Get-Acl -LiteralPath $Snapshot.Path + $acl.SetSecurityDescriptorSddlForm($Snapshot.Access, + [System.Security.AccessControl.AccessControlSections]::Access) + Set-Acl -LiteralPath $Snapshot.Path -AclObject $acl + $restored = (Get-Acl -LiteralPath $Snapshot.Path).GetSecurityDescriptorSddlForm( + [System.Security.AccessControl.AccessControlSections]::Access) + if ($restored -ne $Snapshot.Access) { + throw "The previous access rules on '$($Snapshot.Path)' could not be restored." + } +} + function Assert-ServiceDirectoryAclUsable { <# Verify, repair once, verify again, then fail instead of handing the service a directory it diff --git a/deploy/Build-Artifact.ps1 b/deploy/Build-Artifact.ps1 index 866fd3a8..5c847978 100644 --- a/deploy/Build-Artifact.ps1 +++ b/deploy/Build-Artifact.ps1 @@ -611,6 +611,7 @@ $deployScriptFiles = @( 'Uninstall-NodePilot.ps1' # entry point 'ArtifactSecurity.ps1' # dot-sourced by install + update 'DeploymentProgress.ps1' + 'LocalServerHttp.ps1' 'Preflight.ps1' # dot-sourced by install 'ServiceControl.ps1' # dot-sourced by install + update 'MachinePath.ps1' # dot-sourced by install + update + uninstall diff --git a/deploy/Install-NodePilot.ps1 b/deploy/Install-NodePilot.ps1 index 7d7b6194..691d0449 100644 --- a/deploy/Install-NodePilot.ps1 +++ b/deploy/Install-NodePilot.ps1 @@ -220,6 +220,7 @@ if (-not (Test-Path -LiteralPath $ArtifactSecurityScript -PathType Leaf)) { } . $ArtifactSecurityScript . (Join-Path $PSScriptRoot 'DeploymentProgress.ps1') +. (Join-Path $PSScriptRoot 'LocalServerHttp.ps1') # Readiness checks live in their own file because the setup wizard runs the same set behind a # re-check button. Because of that shared use, nothing in Preflight.ps1 may mutate anything. @@ -250,6 +251,8 @@ $postgresRootCertificatePreviouslyExisted = $false $postgresRootCertificateTouched = $false $installMutationStarted = $false $dataAclApplied = $false +$previousInstallRootAccess = $null +$installRootAclApplied = $false try { if (-not (Test-Path -LiteralPath $ArtifactPath -PathType Leaf)) { throw "Artifact not found: $ArtifactPath" } $ArtifactPath = (Resolve-Path -LiteralPath $ArtifactPath).Path @@ -585,47 +588,19 @@ function Enable-SqlReadCommittedSnapshot { function Invoke-HealthProbe { param( [Parameter(Mandatory)][string]$Url, + [Parameter(Mandatory)][string]$CertificateThumbprint, [int]$TimeoutSeconds = 60 ) - # Trust any cert on localhost - the installer may run before a public hostname resolves, - # and the only caller is the installer itself. - if ($PSVersionTable.PSVersion.Major -ge 6) { - $deadline = (Get-Date).AddSeconds($TimeoutSeconds) - while ((Get-Date) -lt $deadline) { - try { - $r = Invoke-WebRequest -Uri $Url -UseBasicParsing -SkipCertificateCheck -TimeoutSec 5 - if ($r.StatusCode -eq 200) { return $true } - } catch { Start-Sleep -Seconds 2 } - } - return $false - } else { - if (-not ('TrustAllCerts' -as [type])) { - Add-Type @" -using System.Net; using System.Security.Cryptography.X509Certificates; -public class TrustAllCerts : ICertificatePolicy { - public bool CheckValidationResult(ServicePoint s, X509Certificate c, WebRequest r, int p) { return true; } -} -"@ - } - $previousCertificatePolicy = [System.Net.ServicePointManager]::CertificatePolicy - $previousSecurityProtocol = [System.Net.ServicePointManager]::SecurityProtocol + $deadline = (Get-Date).AddSeconds($TimeoutSeconds) + while ((Get-Date) -lt $deadline) { try { - [System.Net.ServicePointManager]::CertificatePolicy = New-Object TrustAllCerts - [System.Net.ServicePointManager]::SecurityProtocol = 'Tls12, Tls13' - $deadline = (Get-Date).AddSeconds($TimeoutSeconds) - while ((Get-Date) -lt $deadline) { - try { - $r = Invoke-WebRequest -Uri $Url -UseBasicParsing -TimeoutSec 5 - if ($r.StatusCode -eq 200) { return $true } - } catch { Start-Sleep -Seconds 2 } - } - return $false - } - finally { - [System.Net.ServicePointManager]::CertificatePolicy = $previousCertificatePolicy - [System.Net.ServicePointManager]::SecurityProtocol = $previousSecurityProtocol + $response = Invoke-NodePilotLocalHttps -Uri $Url -CertificateThumbprint $CertificateThumbprint -TimeoutSeconds 5 + if ($response.StatusCode -eq 200) { return $true } } + catch { } + Start-Sleep -Seconds 2 } + return $false } function Grant-LogOnAsServiceRight { @@ -1008,6 +983,10 @@ if (Test-Path -LiteralPath $previousSettingsPath -PathType Leaf) { $previousSettingsBytes = [IO.File]::ReadAllBytes($previousSettingsPath) } +# The install root grants the service identity execute access. A failed identity change must give +# the previous identity that access back before its restored binaries start. +$previousInstallRootAccess = Get-NodePilotDirectoryAccessSnapshot -Path $InstallPath + if ((Test-Path -LiteralPath $InstallPath -PathType Container) -and @(Get-ChildItem -LiteralPath $InstallPath -Force).Count -gt 0) { Write-Step 'Backing up the existing installation' @@ -1073,6 +1052,7 @@ New-Item -ItemType Directory -Path (Join-Path $DataPath 'logs') -Force | Out-Nul # The binaries are read-only for the service: it executes them, it never rewrites them. # Inheritance is dropped here too, so a permissive parent (a custom root on D:\ inherits # BUILTIN\Users:(M) from the volume) cannot reach the files the service starts from. +$installRootAclApplied = $true Set-DirectoryAclForService -Path $InstallPath -ServiceAccount $AclIdentity ` -ReadOnlyForService -SkipServiceRule:$isLocalSystem Set-DirectoryAclForService -Path $DataPath -ServiceAccount $AclIdentity -SkipServiceRule:$isLocalSystem @@ -1438,7 +1418,7 @@ try { # spamming "false-positive failed" reports for installs that ARE actually succeeding. $probeUrl = "https://localhost:$HttpsPort/healthz/ready" Write-Info " Probing $probeUrl (up to 180s)..." -if (-not (Invoke-HealthProbe -Url $probeUrl -TimeoutSeconds 180)) { +if (-not (Invoke-HealthProbe -Url $probeUrl -CertificateThumbprint $NormalizedThumbprint -TimeoutSeconds 180)) { Show-NodePilotServiceStartDiagnostics -DataPath $DataPath throw "Service did not report /healthz/ready within 180s. Check the NodePilot and Windows Application logs for the startup error." } @@ -1670,6 +1650,12 @@ catch { Remove-ExistingService -Name $ServiceName -InstallPath $InstallPath ` -FoundMessage "Removing the service '$ServiceName' that this run registered." } + # Before the binaries are copied back: restored files inherit this DACL, and the + # previous service identity has to be able to execute them. + if ($installRootAclApplied -and $previousInstallRootAccess -and + (Test-Path -LiteralPath $InstallPath -PathType Container)) { + Restore-NodePilotDirectoryAccessSnapshot -Snapshot $previousInstallRootAccess + } if ($installRollback) { # Refuses a backup whose DACL or files changed since this run made it, before the # install directory is wiped. diff --git a/deploy/Invoke-NodePilotSetup.ps1 b/deploy/Invoke-NodePilotSetup.ps1 index cc710d25..5adbe5ab 100644 --- a/deploy/Invoke-NodePilotSetup.ps1 +++ b/deploy/Invoke-NodePilotSetup.ps1 @@ -135,6 +135,7 @@ function Invoke-NodePilotFirstLogin { Justification = 'Sent as a JSON body and written to the credential file; plaintext throughout by design.')] param( [Parameter(Mandatory)][int]$HttpsPort, + [Parameter(Mandatory)][string]$CertificateThumbprint, [Parameter(Mandatory)][string]$Username, [Parameter(Mandatory)][string]$Password, [Parameter(Mandatory)][string]$Token @@ -144,40 +145,22 @@ function Invoke-NodePilotFirstLogin { $body = @{ username = $Username; password = $Password } | ConvertTo-Json -Compress $headers = @{ 'X-Setup-Token' = $Token } - # The certificate on a loopback call names the public hostname, not localhost, so validation - # is relaxed for this request. The previous policy is restored in the finally block so nothing - # else in this process inherits it. - $previousPolicy = $null - $policyChanged = $false try { - if ($PSVersionTable.PSVersion.Major -lt 6) { - if (-not ('TrustAllCertsBootstrap' -as [type])) { - Add-Type @" -using System.Net; using System.Security.Cryptography.X509Certificates; -public class TrustAllCertsBootstrap : ICertificatePolicy { - public bool CheckValidationResult(ServicePoint s, X509Certificate c, WebRequest r, int p) { return true; } -} -"@ - } - $previousPolicy = [System.Net.ServicePointManager]::CertificatePolicy - [System.Net.ServicePointManager]::CertificatePolicy = New-Object TrustAllCertsBootstrap - [System.Net.ServicePointManager]::SecurityProtocol = 'Tls12, Tls13' - $policyChanged = $true - $response = Invoke-WebRequest -Uri $uri -Method POST -Body $body -Headers $headers ` - -ContentType 'application/json' -UseBasicParsing -TimeoutSec 30 - } - else { - $response = Invoke-WebRequest -Uri $uri -Method POST -Body $body -Headers $headers ` - -ContentType 'application/json' -UseBasicParsing -TimeoutSec 30 -SkipCertificateCheck + $response = Invoke-NodePilotLocalHttps -Uri $uri -CertificateThumbprint $CertificateThumbprint ` + -Method POST -Body $body -Headers $headers -TimeoutSeconds 30 + if ($response.StatusCode -lt 200 -or $response.StatusCode -ge 300) { + throw "Bootstrap login returned HTTP $($response.StatusCode); redirects are not accepted." } return [pscustomobject]@{ Status = 'Created'; Detail = "HTTP $($response.StatusCode)" } } catch { # Report the server's own message: a password-policy rejection or a username mismatch is - # actionable, a generic failure is not. - $detail = $_.Exception.Message + # actionable, a generic failure is not. The .NET call wraps the WebException that carries it. + $webError = $_.Exception + while ($webError -and -not ($webError -is [Net.WebException])) { $webError = $webError.InnerException } + $detail = if ($webError) { $webError.Message } else { $_.Exception.Message } try { - $errorResponse = $_.Exception.Response + $errorResponse = if ($webError) { $webError.Response } else { $null } if ($errorResponse) { $reader = New-Object IO.StreamReader($errorResponse.GetResponseStream()) $payload = $reader.ReadToEnd() @@ -186,9 +169,6 @@ public class TrustAllCertsBootstrap : ICertificatePolicy { } catch { } return [pscustomobject]@{ Status = 'Failed'; Detail = $detail } } - finally { - if ($policyChanged) { [System.Net.ServicePointManager]::CertificatePolicy = $previousPolicy } - } } function Get-NodePilotServiceCrashReason { @@ -694,6 +674,9 @@ function Invoke-SetupInstall { $answers = Read-NodePilotAnswerFile -Path $AnswerFile Set-NodePilotResult -Buffer $result -Section 'result' -Name 'mode' -Value 'install' . (Join-Path $scriptDirectory 'ArtifactSecurity.ps1') + # Only the install mode talks to the local server; loading it compiles C#, so the wizard's + # other modes skip it. + . (Join-Path $scriptDirectory 'LocalServerHttp.ps1') $splat = ConvertTo-NodePilotInstallParameters -Answers $answers @@ -783,6 +766,7 @@ function Invoke-SetupInstall { $bootstrapUser = [string]$answers['bootstrap.adminUsername'] $bootstrapPassword = New-NodePilotBootstrapPassword $outcome = Invoke-NodePilotFirstLogin -HttpsPort ([int]$answers['network.httpsPort']) ` + -CertificateThumbprint $splat['CertThumbprint'] ` -Username $bootstrapUser -Password $bootstrapPassword -Token $token Set-NodePilotResult -Buffer $result -Section 'bootstrap' -Name 'status' -Value $outcome.Status diff --git a/deploy/LocalServerHttp.ps1 b/deploy/LocalServerHttp.ps1 new file mode 100644 index 00000000..e07be4b4 --- /dev/null +++ b/deploy/LocalServerHttp.ps1 @@ -0,0 +1,88 @@ +#requires -Version 5.1 +[CmdletBinding()] +param() + +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version 3.0 + +if (-not ('NodePilot.Deployment.LocalServerHttp' -as [type])) { + # The Framework-compatible HTTP API is obsolete on PowerShell 7, but its per-request + # certificate callback is supported on both runtimes. + Add-Type -IgnoreWarnings -TypeDefinition @' +using System; +using System.Net; +using System.Text.RegularExpressions; + +namespace NodePilot.Deployment +{ + public static class LocalServerHttp + { + public static HttpWebRequest CreateRequest(Uri uri, string certificateThumbprint, int timeoutMilliseconds) + { + if (!uri.IsAbsoluteUri || uri.Scheme != Uri.UriSchemeHttps || !uri.IsLoopback || uri.UserInfo.Length != 0) + throw new ArgumentException("Setup requests require a loopback HTTPS URL.", "uri"); + if (certificateThumbprint == null || !Regex.IsMatch(certificateThumbprint, "\\A[0-9a-fA-F]{40}\\z")) + throw new ArgumentException("The configured server certificate thumbprint is required.", "certificateThumbprint"); + if (timeoutMilliseconds <= 0) + throw new ArgumentOutOfRangeException("timeoutMilliseconds"); + + var request = (HttpWebRequest)WebRequest.Create(uri); + request.AllowAutoRedirect = false; + request.Proxy = null; + request.Timeout = timeoutMilliseconds; + request.ReadWriteTimeout = timeoutMilliseconds; + // Preflight validates the selected certificate. Loopback uses that exact identity, + // even when the certificate names the public hostname or is explicitly self-signed. + request.ServerCertificateValidationCallback = (sender, certificate, chain, errors) => + certificate != null && String.Equals(certificate.GetCertHashString(), certificateThumbprint, StringComparison.OrdinalIgnoreCase); + return request; + } + } +} +'@ +} + +function Get-NodePilotLocalHttpsProtocol { + # Windows PowerShell without the system-default switch offers only SSL 3 and TLS 1.0, which + # Kestrel refuses; TLS 1.3 covers hosts with TLS 1.2 disabled. 0 is SystemDefault: the OS + # already negotiates TLS 1.2 or later. + param([Parameter(Mandatory)][Net.SecurityProtocolType]$Current) + if ([int]$Current -eq 0) { return $Current } + return $Current -bor [Net.SecurityProtocolType]::Tls12 -bor [Net.SecurityProtocolType]::Tls13 +} + +function Invoke-NodePilotLocalHttps { + [CmdletBinding()] + param( + [Parameter(Mandatory)][uri]$Uri, + [Parameter(Mandatory)][string]$CertificateThumbprint, + [ValidateSet('GET', 'POST')][string]$Method = 'GET', + [string]$Body, + [hashtable]$Headers = @{}, + [ValidateRange(1, 300)][int]$TimeoutSeconds = 30 + ) + + $request = [NodePilot.Deployment.LocalServerHttp]::CreateRequest($Uri, $CertificateThumbprint, $TimeoutSeconds * 1000) + $request.Method = $Method + foreach ($key in $Headers.Keys) { $request.Headers.Add([string]$key, [string]$Headers[$key]) } + # The protocol setting is process-wide, so it only applies for this request. + $previousProtocol = [Net.ServicePointManager]::SecurityProtocol + [Net.ServicePointManager]::SecurityProtocol = Get-NodePilotLocalHttpsProtocol -Current $previousProtocol + try { + if ($Method -eq 'POST') { + $request.ContentType = 'application/json' + $bytes = [Text.Encoding]::UTF8.GetBytes($Body) + try { + $request.ContentLength = $bytes.Length + $stream = $request.GetRequestStream() + try { $stream.Write($bytes, 0, $bytes.Length) } + finally { $stream.Dispose() } + } + finally { [Array]::Clear($bytes, 0, $bytes.Length) } + } + $response = $request.GetResponse() + } + finally { [Net.ServicePointManager]::SecurityProtocol = $previousProtocol } + try { return [pscustomobject]@{ StatusCode = [int]$response.StatusCode } } + finally { $response.Dispose() } +} diff --git a/deploy/Preflight.ps1 b/deploy/Preflight.ps1 index d4c36f56..ef252329 100644 --- a/deploy/Preflight.ps1 +++ b/deploy/Preflight.ps1 @@ -1078,6 +1078,18 @@ function Get-NodePilotGmsaRetrievalState { catch { return & $unknown $_.Exception.Message } } +function Get-NodePilotGmsaRetrievalRemediation { + param([Parameter(Mandatory)][string]$Sam, [Parameter(Mandatory)][string]$ComputerName) + $accountLiteral = $Sam.Replace("'", "''") + $computerLiteral = $ComputerName.Replace("'", "''") + @( + ('$account = Get-ADServiceAccount -Identity ''{0}'' -Properties PrincipalsAllowedToRetrieveManagedPassword -ErrorAction Stop' -f $accountLiteral) + '$principals = @($account.PrincipalsAllowedToRetrieveManagedPassword | ForEach-Object { if ($_ -is [string]) { $_ } else { $_.DistinguishedName } })' + ('$principals += (Get-ADComputer -Identity ''{0}'' -ErrorAction Stop).DistinguishedName' -f $computerLiteral) + 'Set-ADServiceAccount -Identity $account -PrincipalsAllowedToRetrieveManagedPassword @($principals | Sort-Object -Unique) -ErrorAction Stop' + ) -join "`r`n" +} + function New-NodePilotGmsaRetrievalResult { <# The verdict for the LDAP answer, separated from the directory call so every branch is @@ -1110,8 +1122,7 @@ function New-NodePilotGmsaRetrievalResult { -RemediationHint ('Allow this host to retrieve the password, then RESTART it - a computer ' + "account's group membership only reaches the machine through a new " + 'Kerberos ticket. As Domain Admin:') ` - -Remediation ("Set-ADServiceAccount -Identity $Sam -PrincipalsAllowedToRetrieveManagedPassword " + - "(Get-ADComputer '$env:COMPUTERNAME')") ` + -Remediation (Get-NodePilotGmsaRetrievalRemediation -Sam $Sam -ComputerName $env:COMPUTERNAME) ` -AbortMessage "$env:COMPUTERNAME may not retrieve the managed password of '$Sam'." } New-NodePilotPreflightResult -Id 'gmsa' -Title $Title -Status 'Pass' ` diff --git a/deploy/README.md b/deploy/README.md index 678a60d0..b461cc64 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -257,24 +257,19 @@ The shipped default profile leaves all external providers disabled and sets loca `BreakGlassOnly`. SCIM tokens are rotated without interruption by keeping the old value briefly under `Authentication:Scim:PreviousBearerToken` and deleting it after the IdP switch. -### 5. Kerberos constrained delegation (for WinRM targets) +### 5. Authorize the WinRM execution identity -> This section concerns the **integrated (credential-less) WinRM path only**. If NodePilot uses credentials stored per machine, no delegation is needed — the service's identity is then irrelevant. Under **LocalSystem** the identity to delegate is the **computer account** of the NodePilot server (`Get-ADComputer `) instead of the gMSA — otherwise the procedure is identical. +The integrated (credential-less) WinRM path authenticates directly as the NodePilot +process identity: the gMSA, or the host computer account when running as LocalSystem. +Grant that identity permission to the target's WinRM endpoint and the resources used +by the workflow. For LocalSystem HA installations, authorize each NodePilot host. +This first hop needs no resource-based constrained delegation and does not impersonate +the browser user. Stored per-machine credentials use their own account permissions. -For the gMSA to reach target machines via implicit Kerberos, **resource-based constrained delegation** -has to be configured on every target server: - -```powershell -# On a DC: -$gmsa = Get-ADServiceAccount -Identity svc-nodepilot -foreach ($target in $targetHosts) { - $computer = Get-ADComputer -Identity $target - Set-ADComputer -Identity $target ` - -PrincipalsAllowedToDelegateToAccount ( - (Get-ADComputer $target).PrincipalsAllowedToDelegateToAccount + $gmsa - ) -} -``` +Delegated-user or second-hop access is a separate topology requiring an explicit design; +do not enable delegation on every target as part of this setup. Existing AD retrieval +or delegation grants must be preserved when an administrator changes them. The setup's +gMSA repair snippet reads existing password-retrieval principals before adding this host. WinRM endpoint on the targets (if not already active): @@ -562,8 +557,15 @@ in the lab: 31 seconds). If the service is deleted in that window, the process i longer reachable through the SCM — and the subsequent deletion pulls the DLLs out from under a running application. The script therefore waits up to `-ProcessExitTimeoutSeconds` (default 90) for processes from `InstallPath` and otherwise aborts **with the service still registered**, so that a supported way -to stop it remains. If files are left behind afterwards (virus scanner, search indexer), the script -still runs to completion, names the leftovers together with the holding process, and exits with code 1. +to stop it remains. A failed `sc.exe delete` stops it the same way, before any file is removed. If files +are left behind afterwards (virus scanner, search indexer), the script still runs to completion, names +the leftovers together with the holding process, and exits with code 1. + +**A failed uninstall can be repeated.** The installation marker and `\deploy\` (which holds +this script) are removed only after everything else is gone. Any non-zero exit makes the GUI and +`/SILENT` uninstaller abort before Inno removes a file or the "Apps & features" entry, and the +uninstaller itself exits non-zero; fix the cause named in `%TEMP%\nodepilot-server-uninstall.log` +(the script's transcript, deleted after a clean run) and uninstall again. Two things are **deliberately** left in place, because both can be shared with something else on the host and blindly revoking them would break exactly that: the gMSA's "Log on as a service" right and the @@ -573,9 +575,11 @@ uninstall whether it is still there. **`-PurgeData` takes ownership first.** The installer writes `jwt-secret.key` and `admin-setup.token` owner-only to the *service account*; otherwise not even an administrator can delete them and the run -aborts halfway. The script therefore calls `takeown` and `icacls` with the well-known SID -`S-1-5-32-544` — **without** `(OI)(CI)`: those are container inheritance flags that are silently -discarded on a leaf file while `icacls` still reports success. +aborts halfway. The script therefore takes ownership item by item with `takeown /A` (the Administrators +group, without the localized confirmation answer `takeown /R /D` expects) and grants the well-known SID +`S-1-5-32-544` with `icacls` — **without** `(OI)(CI)`: those are container inheritance flags that are +silently discarded on a leaf file while `icacls` still reports success. If either tool fails for any +item, nothing under the data directory is deleted and the run exits with code 1. ## Backup & disaster recovery diff --git a/deploy/Test-DeploymentTemplates.ps1 b/deploy/Test-DeploymentTemplates.ps1 index 9cefb1fc..1ee0a1d1 100644 --- a/deploy/Test-DeploymentTemplates.ps1 +++ b/deploy/Test-DeploymentTemplates.ps1 @@ -1023,6 +1023,54 @@ Assert-TextMatches -Name 'a failed first install is narrated as an undo' ` # wording written for an upgrade. Assert-TextMatches -Name 'the rollback names the service it registered itself' ` -Text $rollbackBlock -Pattern "FoundMessage `"Removing the service '\`$ServiceName' that this run registered\.`"" +# The install root carries the execute grant for the service identity. A failed gMSA A-to-B +# reinstall restored A's binaries under B's DACL, so A could not start them. +Assert-TextMatches -Name 'the installer snapshots the install root DACL before changing it' ` + -Text $installerScript -Pattern '(?s)\$previousInstallRootAccess = Get-NodePilotDirectoryAccessSnapshot -Path \$InstallPath.*?\$installRootAclApplied = \$true\s*\r?\nSet-DirectoryAclForService -Path \$InstallPath' +Assert-TextMatches -Name 'the rollback restores install root access before the binaries and the service' ` + -Text $rollbackBlock -Pattern '(?s)Restore-NodePilotDirectoryAccessSnapshot -Snapshot \$previousInstallRootAccess.*?Restore-NodePilotBinaryBackup -Backup \$installRollback.*?Restore-ServiceRollbackSnapshot -Snapshot \$previousService' +& { + . $ArtifactSecurityPath + $root = Join-Path ([IO.Path]::GetTempPath()) ('nodepilot-installroot-' + [Guid]::NewGuid().ToString('N')) + New-Item -ItemType Directory -Path $root | Out-Null + # LocalService and NetworkService stand in for gMSA A and gMSA B. + $identityA = New-Object System.Security.Principal.SecurityIdentifier('S-1-5-19') + $identityB = New-Object System.Security.Principal.SecurityIdentifier('S-1-5-20') + $me = [System.Security.Principal.WindowsIdentity]::GetCurrent().User + $setRootAcl = { + param($ServiceIdentity) + $acl = New-Object System.Security.AccessControl.DirectorySecurity + $acl.SetAccessRuleProtection($true, $false) + foreach ($grant in @(@($me, 'FullControl'), @($ServiceIdentity, 'ReadAndExecute'))) { + $acl.AddAccessRule((New-Object System.Security.AccessControl.FileSystemAccessRule( + $grant[0], $grant[1], 'ContainerInherit,ObjectInherit', 'None', 'Allow'))) + } + Set-Acl -LiteralPath $root -AclObject $acl + } + $grantsTo = { + param($Path, $Sid) + @((Get-Acl -LiteralPath $Path).Access | Where-Object { + $_.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]) -eq $Sid -and + ($_.FileSystemRights -band [System.Security.AccessControl.FileSystemRights]::ExecuteFile) + }).Count -gt 0 + } + try { + & $setRootAcl $identityA + $snapshot = Get-NodePilotDirectoryAccessSnapshot -Path $root + & $setRootAcl $identityB + Restore-NodePilotDirectoryAccessSnapshot -Snapshot $snapshot + [IO.File]::WriteAllText((Join-Path $root 'NodePilot.Api.exe'), 'restored binary') + $restored = Join-Path $root 'NodePilot.Api.exe' + if (-not (& $grantsTo $restored $identityA) -or (& $grantsTo $restored $identityB)) { + throw 'Deployment template check failed: a restored binary must be executable by the previous identity only.' + } + if ($null -ne (Get-NodePilotDirectoryAccessSnapshot -Path (Join-Path $root 'absent'))) { + throw 'Deployment template check failed: an absent install root has no access snapshot.' + } + } + finally { Remove-Item -LiteralPath $root -Recurse -Force -ErrorAction SilentlyContinue } + Write-Host ' OK a failed identity change gives the previous identity its install root access back' -ForegroundColor DarkGray +} Assert-TextMatches -Name 'the rollback hands the secrets back to the previous identity too' ` -Text $rollbackBlock -Pattern '(?s)Set-NodePilotServiceOwnedFileAcl[\s\S]{0,200}\$previousAclIdentity' # Guarded, so a failure before the ACL was ever touched does not rewrite a directory this run had @@ -1203,17 +1251,108 @@ if ($purgeBlockStart -lt 0) { } $purgeBlock = $uninstallScript.Substring($purgeBlockStart) Assert-TextMatches -Name 'purging takes ownership before deleting owner-only files' ` - -Text $purgeBlock -Pattern 'takeown\.exe' -# "BUILTIN\Administrators" does not resolve on a non-English Windows. + -Text $purgeBlock -Pattern '(?s)Grant-NodePilotPurgeAccess -Path \$DataPath.*?Remove-Item -LiteralPath \$DataPath' +$purgeAccessStart = $uninstallScript.IndexOf('function Grant-NodePilotPurgeAccess') +if ($purgeAccessStart -lt 0) { + throw 'Deployment template check failed: could not locate the purge ownership helper in the uninstaller.' +} +$purgeAccess = $uninstallScript.Substring($purgeAccessStart, + $uninstallScript.IndexOf("`nWrite-Step", $purgeAccessStart) - $purgeAccessStart) +# "BUILTIN\Administrators" does not resolve on a non-English Windows, and takeown /D expects a +# localized answer (Y on English, J on German Windows). /A names the group without either. +Assert-TextMatches -Name 'ownership goes to Administrators without a localized name' ` + -Text $purgeAccess -Pattern 'takeown\.exe /F \$ItemPath /A' +Assert-TextDoesNotMatch -Name 'takeown is never given a localized confirmation answer' ` + -Text $uninstallScript -Pattern 'takeown[^\r\n]*/D\b' Assert-TextMatches -Name 'the ownership grant uses the well-known SID, not a localised group name' ` - -Text $purgeBlock -Pattern 'S-1-5-32-544' + -Text $purgeAccess -Pattern 'S-1-5-32-544' Assert-TextDoesNotMatch -Name 'the ownership grant must not use a localised group name' ` - -Text $purgeBlock -Pattern 'BUILTIN\\Administrators' + -Text $purgeAccess -Pattern 'BUILTIN\\Administrators' # (OI) and (CI) are CONTAINER inheritance flags. Applied to a leaf file, icacls drops them, reports # "Successfully processed 1 files" and adds no ACE whatsoever - so the grant looks like it worked # and the file stays undeletable. Measured on the lab host against jwt-secret.key. Assert-TextDoesNotMatch -Name 'the grant must not carry container inheritance flags' ` - -Text $purgeBlock -Pattern 'icacls[^\r\n]*\(OI\)' + -Text $purgeAccess -Pattern 'icacls[^\r\n]*\(OI\)' + +# A failed native step leaves the data in place and fails the uninstall instead of deleting part +# of a tree it could not open. +Assert-TextMatches -Name 'ownership failures stop the purge before anything is deleted' ` + -Text $purgeBlock -Pattern '(?s)if \(\$ownershipFailures\.Count -gt 0\) \{.*?\$dataPathRemaining = \$true.*?\}\s*else \{\s*try \{\s*Remove-Item -LiteralPath \$DataPath' + +# The helper against a real tree with a folder this account cannot list until its "ownership" is +# taken, which is what an owner-only directory looks like to an administrator. +& { + $uninstallErrors = $null + $uninstallAst = [System.Management.Automation.Language.Parser]::ParseFile( + $UninstallScriptPath, [ref]$null, [ref]$uninstallErrors) + if ($uninstallErrors.Count) { throw 'Uninstaller contains PowerShell syntax errors.' } + $helper = $uninstallAst.Find({ param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and + $node.Name -eq 'Grant-NodePilotPurgeAccess' + }, $false) + . ([scriptblock]::Create($helper.Extent.Text)) + $fixture = Join-Path ([IO.Path]::GetTempPath()) ('nodepilot-purge-' + [Guid]::NewGuid().ToString('N')) + $locked = Join-Path $fixture 'keys' + New-Item -ItemType Directory -Path (Join-Path $locked 'inner') -Force | Out-Null + foreach ($file in @('jwt-secret.key', 'keys\key-1.xml', 'keys\inner\key-2.xml')) { + [IO.File]::WriteAllText((Join-Path $fixture $file), 'fixture') + } + $me = [System.Security.Principal.WindowsIdentity]::GetCurrent().User + $deny = New-Object System.Security.AccessControl.FileSystemAccessRule($me, 'ListDirectory', 'Deny') + $lockedAcl = Get-Acl -LiteralPath $locked + $lockedAcl.AddAccessRule($deny) + Set-Acl -LiteralPath $locked -AclObject $lockedAcl + $calls = New-Object 'System.Collections.Generic.List[string]' + $unlock = { + param($ItemPath) + $calls.Add("own:$ItemPath") + if ($ItemPath -eq $locked) { + $acl = Get-Acl -LiteralPath $locked + [void]$acl.RemoveAccessRule($deny) + Set-Acl -LiteralPath $locked -AclObject $acl + } + 0 + } + try { + $failures = @(Grant-NodePilotPurgeAccess -Path $fixture -TakeOwnership $unlock ` + -GrantAdministrators { param($ItemPath) $calls.Add("grant:$ItemPath"); 0 }) + $expected = @($fixture, (Join-Path $fixture 'jwt-secret.key'), $locked, (Join-Path $locked 'inner'), + (Join-Path $locked 'key-1.xml'), (Join-Path $locked 'inner\key-2.xml')) + $missing = @($expected | Where-Object { + $calls.IndexOf("own:$_") -lt 0 -or $calls.IndexOf("grant:$_") -lt $calls.IndexOf("own:$_") }) + if ($failures.Count -ne 0 -or $missing.Count -ne 0) { + throw "Deployment template check failed: purge ownership must reach every item behind an unlistable folder (missing: $($missing -join ', '))." + } + + $calls.Clear() + $failures = @(Grant-NodePilotPurgeAccess -Path $fixture ` + -TakeOwnership { param($ItemPath) $calls.Add("own:$ItemPath"); if ($ItemPath -like '*jwt-secret.key') { 1 } else { 0 } } ` + -GrantAdministrators { param($ItemPath) $calls.Add("grant:$ItemPath"); 0 }) + if ($failures.Count -ne 1 -or $failures[0] -notmatch 'takeown exited with 1' -or + $calls.Contains("grant:$(Join-Path $fixture 'jwt-secret.key')")) { + throw 'Deployment template check failed: a failed takeown must be reported and must skip the grant.' + } + } + finally { + $acl = Get-Acl -LiteralPath $locked + [void]$acl.RemoveAccessRule($deny) + Set-Acl -LiteralPath $locked -AclObject $acl + Remove-Item -LiteralPath $fixture -Recurse -Force -ErrorAction SilentlyContinue + } + Write-Host ' OK purge ownership is taken item by item without a localized answer and reports native failures' -ForegroundColor DarkGray +} + +# A failed cleanup must stay repeatable: the marker and deploy\ are removed only after everything +# else is gone, and both setup uninstallers abort before Inno removes anything. +Assert-TextMatches -Name 'the uninstaller keeps the marker and its scripts while anything is left' ` + -Text $uninstallScript -Pattern '(?s)if \(\$installPathRemaining -or \$backupsRemaining -or \$dataPathRemaining\) \{.*?exit 1\s*\}.*?Remove-Item -LiteralPath \$recoveryHelperPath.*?Remove-Item -LiteralPath \$markerPath' +Assert-TextMatches -Name 'the uninstaller defers deploy\ while it empties the install directory' ` + -Text $uninstallScript -Pattern "\`$entry\.Name -eq \`$recoveryHelperName\) \{ continue \}" +Assert-TextMatches -Name 'a failed service deletion stops the uninstall before files are removed' ` + -Text $uninstallScript -Pattern '(?s)\$deleteExit -ne 0 -and \$deleteExit -ne 1072\) \{\s*throw' +# The setup runs the script hidden, so a stopped uninstall is diagnosable only from its transcript. +Assert-TextMatches -Name 'the uninstaller keeps a transcript until a clean run deletes it' ` + -Text $uninstallScript -Pattern '(?s)\$TranscriptPath = Join-Path \$env:TEMP ''nodepilot-server-uninstall\.log''\s*\r?\ntry \{ Start-Transcript -Path \$TranscriptPath.*?Write-Ok "Uninstall complete\."\s*\r?\ntry \{ Stop-Transcript \| Out-Null \} catch \{ \}\s*\r?\nRemove-Item -LiteralPath \$TranscriptPath' # --- server setup wizard contracts ------------------------------------------------------------- # The Pascal side has no unit-test story at all, so what CAN be pinned statically is pinned here @@ -1225,6 +1364,18 @@ Assert-TextDoesNotMatch -Name 'the server setup must not use a [Run] section' ` -Text $serverIss -Pattern '(?mi)^\s*\[Run\]' Assert-TextMatches -Name 'every Exec result is inspected' ` -Text $serverIss -Pattern '(?m)ResultCode\s*<>\s*0' +# Abort in usUninstall is the one place Inno stops an uninstall before it removes files and the +# Apps & Features entry, with a non-zero exit code for /SILENT callers. +$serverUninstallStep = [regex]::Match($serverIss, + '(?s)procedure CurUninstallStepChanged\(CurUninstallStep: TUninstallStep\);.*?\r?\nend;').Value +Assert-TextMatches -Name 'a failed server cleanup aborts the uninstall' ` + -Text $serverUninstallStep -Pattern '(?s)else if ResultCode <> 0 then\s*begin.*?Abort;\s*end;' +Assert-TextMatches -Name 'a failed server cleanup names the uninstall transcript' ` + -Text $serverUninstallStep -Pattern "(?s)else if ResultCode <> 0 then\s*begin.*?GetEnv\('TEMP'\) \+ '\\nodepilot-server-uninstall\.log'.*?Abort;" +Assert-TextMatches -Name 'a server cleanup that cannot start aborts the uninstall' ` + -Text $serverUninstallStep -Pattern "(?s)if not Exec\('powershell\.exe', Arguments.*?then\s*begin.*?Abort;\s*end" +Assert-TextMatches -Name 'a missing server cleanup script aborts while the installation marker exists' ` + -Text $serverUninstallStep -Pattern "(?s)if not FileExists\(ScriptPath\) then\s*begin\s*//[^\r\n]*\r?\n\s*//[^\r\n]*\r?\n\s*if not RegKeyExists\(HKLM64, 'SOFTWARE\\NodePilot\\Server'\) then Exit;.*?Abort;" # Windows Server 2022 is build 20348. Copying the desktop installer's 22000 would make the SERVER # setup refuse to run on the only operating system it targets. diff --git a/deploy/Test-LocalServerHttp.ps1 b/deploy/Test-LocalServerHttp.ps1 new file mode 100644 index 00000000..a74b5fb5 --- /dev/null +++ b/deploy/Test-LocalServerHttp.ps1 @@ -0,0 +1,127 @@ +#requires -Version 5.1 +[CmdletBinding()] +param() + +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version 3.0 +. (Join-Path $PSScriptRoot 'LocalServerHttp.ps1') + +$script:passed = 0 +function Assert-LocalHttps { + param([bool]$Condition, [string]$Message) + if (-not $Condition) { throw $Message } + $script:passed++ +} + +function Import-SetupFunction { + param([string]$FileName, [string]$Name) + $tokens = $null; $errors = $null + $ast = [Management.Automation.Language.Parser]::ParseFile((Join-Path $PSScriptRoot $FileName), [ref]$tokens, [ref]$errors) + if ($errors.Count) { throw "Could not parse $FileName" } + $function = $ast.Find({ param($node) $node -is [Management.Automation.Language.FunctionDefinitionAst] -and $node.Name -eq $Name }, $true) + if (-not $function) { throw "Missing function $Name" } + return [scriptblock]::Create($function.Extent.Text) +} + +$rsa = [Security.Cryptography.RSA]::Create(2048) +$certificate = $null; $otherCertificate = $null +try { + $certificateRequest = [Security.Cryptography.X509Certificates.CertificateRequest]::new( + 'CN=setup.synthetic.invalid', $rsa, [Security.Cryptography.HashAlgorithmName]::SHA256, [Security.Cryptography.RSASignaturePadding]::Pkcs1) + $certificate = $certificateRequest.CreateSelfSigned([DateTimeOffset]::UtcNow.AddDays(-1), [DateTimeOffset]::UtcNow.AddDays(1)) + $otherCertificate = $certificateRequest.CreateSelfSigned([DateTimeOffset]::UtcNow.AddDays(-2), [DateTimeOffset]::UtcNow.AddDays(2)) + $policyProperty = [Net.ServicePointManager].GetProperty('CertificatePolicy') + $previousPolicy = if ($policyProperty) { $policyProperty.GetValue($null) } else { $null } + $previousCallback = [Net.ServicePointManager]::ServerCertificateValidationCallback + $request = [NodePilot.Deployment.LocalServerHttp]::CreateRequest([uri]'https://localhost:12345/healthz/ready', $certificate.Thumbprint, 1000) + $errors = [Net.Security.SslPolicyErrors]::RemoteCertificateNameMismatch -bor [Net.Security.SslPolicyErrors]::RemoteCertificateChainErrors + Assert-LocalHttps ($request.ServerCertificateValidationCallback.Invoke($request, $certificate, $null, $errors)) 'The selected self-signed public-host certificate must identify the loopback server.' + Assert-LocalHttps (-not $request.ServerCertificateValidationCallback.Invoke($request, $otherCertificate, $null, [Net.Security.SslPolicyErrors]::None)) 'A different certificate must fail even with a valid chain and hostname.' + Assert-LocalHttps (-not $request.ServerCertificateValidationCallback.Invoke($request, $null, $null, $errors)) 'A missing certificate must fail.' + Assert-LocalHttps (-not $request.AllowAutoRedirect) 'Bootstrap credentials must not follow redirects.' + Assert-LocalHttps ($null -eq $request.Proxy) 'Loopback setup must not use a configured proxy.' + if ($policyProperty) { + Assert-LocalHttps ([object]::ReferenceEquals($previousPolicy, $policyProperty.GetValue($null))) 'The global certificate policy changed.' + } + Assert-LocalHttps ([object]::ReferenceEquals($previousCallback, [Net.ServicePointManager]::ServerCertificateValidationCallback)) 'The global validation callback changed.' + foreach ($uri in @('http://localhost:12345/', 'https://server.synthetic.invalid/', 'https://user@localhost/')) { + $rejected = $false + try { [NodePilot.Deployment.LocalServerHttp]::CreateRequest([uri]$uri, $certificate.Thumbprint, 1000) | Out-Null } + catch { $rejected = $true } + Assert-LocalHttps $rejected "Unsafe setup URI accepted: $uri" + } + $rejected = $false + try { [NodePilot.Deployment.LocalServerHttp]::CreateRequest([uri]'https://localhost/', '', 1000) | Out-Null } + catch { $rejected = $true } + Assert-LocalHttps $rejected 'Missing certificate pin was accepted.' + + # Windows PowerShell without the system-default switch would offer only TLS 1.0 to Kestrel. + $legacy = [Net.SecurityProtocolType]::Tls + $offered = Get-NodePilotLocalHttpsProtocol -Current $legacy + Assert-LocalHttps (($offered -band [Net.SecurityProtocolType]::Tls12) -and ($offered -band [Net.SecurityProtocolType]::Tls13) -and + ($offered -band $legacy)) 'TLS 1.2 and 1.3 must be added to a legacy protocol setting.' + Assert-LocalHttps ([int](Get-NodePilotLocalHttpsProtocol -Current ([Net.SecurityProtocolType]0)) -eq 0) 'The system default must be left to the OS.' + $previousProtocol = [Net.ServicePointManager]::SecurityProtocol + $closedProbe = [System.Net.Sockets.TcpListener]::new([System.Net.IPAddress]::Loopback, 0) + $closedProbe.Start() + $closedPort = ([System.Net.IPEndPoint]$closedProbe.LocalEndpoint).Port + $closedProbe.Stop() + try { + [Net.ServicePointManager]::SecurityProtocol = $legacy + $failed = $false + try { Invoke-NodePilotLocalHttps -Uri "https://localhost:$closedPort/healthz/ready" -CertificateThumbprint $certificate.Thumbprint -TimeoutSeconds 2 | Out-Null } + catch { $failed = $true } + Assert-LocalHttps $failed 'A closed port must fail the request.' + Assert-LocalHttps ([Net.ServicePointManager]::SecurityProtocol -eq $legacy) 'The process-wide protocol setting must be restored after the request.' + } + finally { [Net.ServicePointManager]::SecurityProtocol = $previousProtocol } + + . (Import-SetupFunction -FileName 'Install-NodePilot.ps1' -Name 'Invoke-HealthProbe') + . (Import-SetupFunction -FileName 'Invoke-NodePilotSetup.ps1' -Name 'Invoke-NodePilotFirstLogin') + $script:requests = @() + $script:statusCode = 200 + $script:failWith = $null + function Invoke-NodePilotLocalHttps { + param($Uri, $CertificateThumbprint, $Method = 'GET', $Body, $Headers, $TimeoutSeconds) + $script:requests += [pscustomobject]@{Uri=$Uri; Pin=$CertificateThumbprint; Method=$Method; Body=$Body; Headers=$Headers} + if ($script:failWith) { throw $script:failWith } + return [pscustomobject]@{StatusCode=$script:statusCode} + } + Assert-LocalHttps (Invoke-HealthProbe -Url 'https://localhost:12345/healthz/ready' -CertificateThumbprint $certificate.Thumbprint) 'Pinned readiness response was rejected.' + $result = Invoke-NodePilotFirstLogin -HttpsPort 12345 -CertificateThumbprint $certificate.Thumbprint -Username 'synthetic-admin' -Password 'synthetic-password' -Token 'synthetic-token' + Assert-LocalHttps ($result.Status -eq 'Created') 'Pinned bootstrap response was rejected.' + Assert-LocalHttps (@($script:requests | Where-Object Pin -ne $certificate.Thumbprint).Count -eq 0) 'A caller lost the configured certificate pin.' + Assert-LocalHttps ($script:requests[1].Headers['X-Setup-Token'] -eq 'synthetic-token') 'Bootstrap token was not sent through the pinned transport.' + $script:statusCode = 302 + $result = Invoke-NodePilotFirstLogin -HttpsPort 12345 -CertificateThumbprint $certificate.Thumbprint -Username 'synthetic-admin' -Password 'synthetic-password' -Token 'synthetic-token' + Assert-LocalHttps ($result.Status -eq 'Failed') 'Redirect was treated as successful bootstrap.' + + # A rejected login reaches the caller as a WebException wrapped by the .NET method call; the + # server's message must still be reported. + if (-not ('NodePilot.Deployment.Tests.SyntheticErrorResponse' -as [type])) { + Add-Type -IgnoreWarnings -TypeDefinition @' +namespace NodePilot.Deployment.Tests +{ + public sealed class SyntheticErrorResponse : System.Net.WebResponse + { + private readonly byte[] _body; + public SyntheticErrorResponse(string body) { _body = System.Text.Encoding.UTF8.GetBytes(body); } + public override System.IO.Stream GetResponseStream() { return new System.IO.MemoryStream(_body); } + } +} +'@ + } + $serverMessage = '{"error":"synthetic password policy rejection"}' + $webError = New-Object Net.WebException('The remote server returned an error: (400) Bad Request.', $null, + [Net.WebExceptionStatus]::ProtocolError, (New-Object NodePilot.Deployment.Tests.SyntheticErrorResponse($serverMessage))) + $script:failWith = New-Object Management.Automation.MethodInvocationException('Exception calling "GetResponse" with "0" argument(s)', $webError) + $result = Invoke-NodePilotFirstLogin -HttpsPort 12345 -CertificateThumbprint $certificate.Thumbprint -Username 'synthetic-admin' -Password 'synthetic-password' -Token 'synthetic-token' + Assert-LocalHttps ($result.Status -eq 'Failed' -and $result.Detail -eq $serverMessage) 'A rejected bootstrap login must report the server message.' + $script:failWith = $null +} +finally { + if ($certificate) { $certificate.Dispose() } + if ($otherCertificate) { $otherCertificate.Dispose() } + $rsa.Dispose() +} +Write-Host "Local server HTTPS checks passed: $script:passed" diff --git a/deploy/Test-SetupAdapter.ps1 b/deploy/Test-SetupAdapter.ps1 index d90f5386..44f18d66 100644 --- a/deploy/Test-SetupAdapter.ps1 +++ b/deploy/Test-SetupAdapter.ps1 @@ -1801,6 +1801,29 @@ try { -State ([pscustomobject]@{ Determined = $true; Allowed = $true; Detail = '' }) Assert-True -Name 'a host that may fetch the password passes' -Condition ($permitted.Status -eq 'Pass') + & { + # Execute the generated advice against in-memory AD boundaries only. + function Get-ADServiceAccount { + param($Identity, $Properties, $ErrorAction) + if ($Properties -ne 'PrincipalsAllowedToRetrieveManagedPassword') { throw 'Existing grants were not requested.' } + [pscustomobject]@{ PrincipalsAllowedToRetrieveManagedPassword = @( + [pscustomobject]@{ DistinguishedName = 'CN=ExistingGroup,DC=test' }, 'CN=ExistingHost,DC=test') } + } + function Get-ADComputer { + param($Identity, $ErrorAction) + [pscustomobject]@{ DistinguishedName = 'CN=NewHost,DC=test' } + } + function Set-ADServiceAccount { + param($Identity, $PrincipalsAllowedToRetrieveManagedPassword, $ErrorAction) + Assert-True -Name 'gMSA advice preserves existing grants while adding the host' -Condition ( + @($PrincipalsAllowedToRetrieveManagedPassword).Count -eq 3 -and + $PrincipalsAllowedToRetrieveManagedPassword -contains 'CN=ExistingGroup,DC=test' -and + $PrincipalsAllowedToRetrieveManagedPassword -contains 'CN=ExistingHost,DC=test' -and + $PrincipalsAllowedToRetrieveManagedPassword -contains 'CN=NewHost,DC=test') + } + & ([scriptblock]::Create($denied.Remediation)) + } + # Not knowing is not evidence: an unreachable directory must not refuse an install. $unknown = New-NodePilotGmsaRetrievalResult -Sam 'q-sdvorch2' -Title $gmsaTitle ` -State ([pscustomobject]@{ Determined = $false; Allowed = $false; Detail = 'no directory could be reached' }) diff --git a/deploy/Uninstall-NodePilot.ps1 b/deploy/Uninstall-NodePilot.ps1 index 87a7f4fe..14f2a1eb 100644 --- a/deploy/Uninstall-NodePilot.ps1 +++ b/deploy/Uninstall-NodePilot.ps1 @@ -44,6 +44,10 @@ param( $ErrorActionPreference = 'Stop' Set-StrictMode -Version 3.0 +# The setup uninstaller runs this hidden and names this file when it stops; a clean run deletes it. +$TranscriptPath = Join-Path $env:TEMP 'nodepilot-server-uninstall.log' +try { Start-Transcript -Path $TranscriptPath -Force | Out-Null } catch { } + function Write-Step { param([string]$Text) Write-Host "[uninstall] $Text" -ForegroundColor Cyan } function Write-Info { param([string]$Text) Write-Host "[uninstall] $Text" -ForegroundColor Gray } function Write-Ok { param([string]$Text) Write-Host "[uninstall] $Text" -ForegroundColor Green } @@ -147,6 +151,60 @@ function Remove-NodePilotBinaryBackups { return $failed } +function Grant-NodePilotPurgeAccess { + <# + Takes ownership of $Path and everything below it for Administrators (takeown /A), then grants + that group full control, item by item. takeown /R prompts with a localized answer on folders + it cannot list, so the recursion happens here: a folder unlocked in one pass exposes its + children to the next. A failed takeown skips the grant for that item. Returns the failures. + #> + param( + [Parameter(Mandatory)][string]$Path, + [scriptblock]$TakeOwnership = { + param($ItemPath) + & takeown.exe /F $ItemPath /A 2>&1 | Out-Null + $LASTEXITCODE + }, + # No (OI)(CI): those are container flags, and on a leaf file icacls drops them and adds no + # ACE at all while still reporting success. + [scriptblock]$GrantAdministrators = { + param($ItemPath) + & icacls.exe $ItemPath /grant '*S-1-5-32-544:(F)' /C /Q 2>&1 | Out-Null + $LASTEXITCODE + }, + [int]$MaxPasses = 16 + ) + # Native tools report on stderr; under 'Stop' Windows PowerShell turns that into an exception. + $ErrorActionPreference = 'Continue' + $done = New-Object 'System.Collections.Generic.HashSet[string]' ([StringComparer]::OrdinalIgnoreCase) + $failures = @{} + for ($pass = 1; $pass -le $MaxPasses; $pass++) { + $items = @(Get-Item -LiteralPath $Path -Force -ErrorAction SilentlyContinue) + + @(Get-ChildItem -LiteralPath $Path -Recurse -Force -ErrorAction SilentlyContinue) + $pending = @($items | Where-Object { -not $done.Contains($_.FullName) }) + if ($pending.Count -eq 0) { break } + $progress = $false + # Parents first, so their children become listable for the next pass. + foreach ($item in @($pending | Sort-Object { $_.FullName.Length })) { + $ownerExit = & $TakeOwnership $item.FullName + if ($ownerExit -ne 0) { + $failures[$item.FullName] = "takeown exited with $ownerExit for $($item.FullName)" + continue + } + $grantExit = & $GrantAdministrators $item.FullName + if ($grantExit -ne 0) { + $failures[$item.FullName] = "icacls exited with $grantExit for $($item.FullName)" + continue + } + $failures.Remove($item.FullName) + [void]$done.Add($item.FullName) + $progress = $true + } + if (-not $progress) { break } + } + return @($failures.Values | Sort-Object) +} + Write-Step "Stopping and removing service '$ServiceName'" $svc = Get-Service -Name $ServiceName -ErrorAction SilentlyContinue if ($svc) { @@ -188,7 +246,14 @@ if ($svc) { } & sc.exe delete $ServiceName | Out-Null - Write-Info " sc.exe delete returned exit $LASTEXITCODE" + $deleteExit = $LASTEXITCODE + Write-Info " sc.exe delete returned exit $deleteExit" + # 1072: already marked for deletion. Anything else leaves the service registered, so stop + # before removing the files it runs from. + if ($deleteExit -ne 0 -and $deleteExit -ne 1072) { + throw ("sc.exe delete '$ServiceName' failed with exit $deleteExit. Nothing else was removed; " + + 'fix the cause and run the uninstall again.') + } # sc.exe delete normally removes the whole service key, including the Environment MULTI_SZ # that holds ConnectionStrings__Postgres - the database password. While anything still holds @@ -204,24 +269,6 @@ if ($svc) { Write-Info " Service not present." } -Write-Step "Removing the installation marker" -# Without this, a fresh install after an uninstall is misdetected as an upgrade forever - the -# setup wizard reads this key to choose between the two. -$markerPath = 'HKLM:\SOFTWARE\NodePilot\Server' -if (Test-Path -LiteralPath $markerPath) { - Remove-Item -LiteralPath $markerPath -Recurse -Force -ErrorAction SilentlyContinue - Write-Info " Removed: $markerPath" -} else { - Write-Info " No installation marker present." -} -# Drop the parent only when this uninstall emptied it; something else may live under it. -$markerParent = 'HKLM:\SOFTWARE\NodePilot' -if ((Test-Path -LiteralPath $markerParent) -and - -not (Get-ChildItem -LiteralPath $markerParent -ErrorAction SilentlyContinue) -and - -not (Get-Item -LiteralPath $markerParent -ErrorAction SilentlyContinue).GetValueNames()) { - Remove-Item -LiteralPath $markerParent -Force -ErrorAction SilentlyContinue -} - Write-Step "Removing the CLI from the machine PATH" # Install-NodePilot.ps1 appends \tools\np so operators can just type `np`. Leaving it # behind would point PATH at a directory this uninstall is about to delete, and repeated @@ -260,10 +307,13 @@ if (Test-Path $InstallPath) { # arbitrary remainder behind. Skipping Inno's files lets it finish the job itself; run # standalone there are no such files and everything goes. $uninstallerPattern = 'unins*' + # deploy\ holds this script. It goes last, once everything else is gone, so a failed run can + # be repeated from the same place. + $recoveryHelperName = 'deploy' for ($attempt = 1; $attempt -le 3; $attempt++) { $failed = $false foreach ($entry in Get-ChildItem -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue) { - if ($entry.Name -like $uninstallerPattern) { continue } + if ($entry.Name -like $uninstallerPattern -or $entry.Name -eq $recoveryHelperName) { continue } try { Remove-Item -LiteralPath $entry.FullName -Recurse -Force -ErrorAction Stop } catch { $failed = $true @@ -277,23 +327,17 @@ if (Test-Path $InstallPath) { if ($attempt -lt 3) { Start-Sleep -Seconds 2 } } - # Only when nothing is left. Inno removes its own uninstaller and the empty directory after - # this script returns. - $remaining = @(Get-ChildItem -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue) - if ($remaining.Count -eq 0) { - Remove-Item -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue - } - elseif (@($remaining | Where-Object { $_.Name -notlike $uninstallerPattern }).Count -eq 0) { - Write-Info ' Leaving the setup uninstaller to remove itself and the directory.' - } - - # Anything still here that is not the setup's own uninstaller is a genuine leftover. + # Anything still here that is not the setup's own uninstaller or deploy\ is a genuine leftover. + $recoveryHelperPrefix = (Join-Path $InstallPath $recoveryHelperName).TrimEnd('\') + '\' $leftovers = @(Get-ChildItem -LiteralPath $InstallPath -Recurse -Force -File -ErrorAction SilentlyContinue | - Where-Object { $_.Name -notlike $uninstallerPattern }) + Where-Object { + $_.Name -notlike $uninstallerPattern -and + -not $_.FullName.StartsWith($recoveryHelperPrefix, [StringComparison]::OrdinalIgnoreCase) + }) if ($leftovers.Count -gt 0) { - # Do not abort here. The service and the marker are already gone; stopping now would skip - # the data notice and the residue report. Report the leftovers, carry on, and exit - # non-zero at the end. + # Do not abort here. The service is already gone; stopping now would skip the data notice + # and the residue report. Report the leftovers, carry on, and exit non-zero at the end, + # with the marker and deploy\ kept for the retry. $installPathRemaining = $true Write-Warn " $($leftovers.Count) file(s) could not be deleted under $InstallPath." $holders = @(Get-ProcessesUnderPath -Path $InstallPath) @@ -324,32 +368,30 @@ if ($PurgeData) { # account, so an administrator can neither read nor delete them and a plain Remove-Item # gets partway through the directory and then throws. # - # Ownership is taken first, using the well-known Administrators SID rather than the group - # name: "BUILTIN\Administrators" does not resolve on a non-English Windows, the same - # reason the ACL helpers in ArtifactSecurity.ps1 use SIDs. - # - # The grant carries no inheritance flags. (OI)(CI) are container flags; applied to a leaf - # file icacls drops them silently and adds no ACE at all. /T visits every item, so a flat - # grant is sufficient for a tree that is about to be deleted anyway. - $previousPreference = $ErrorActionPreference - $ErrorActionPreference = 'Continue' - try { - & takeown.exe /F $DataPath /R /D Y 2>&1 | Out-Null - & icacls.exe $DataPath /grant '*S-1-5-32-544:(F)' /T /C 2>&1 | Out-Null - } - finally { $ErrorActionPreference = $previousPreference } - - try { - Remove-Item -LiteralPath $DataPath -Recurse -Force -ErrorAction Stop - Write-Info " Deleted: $DataPath" - } - catch { - # Report and continue. Aborting here would skip the closing report, and the operator - # would be left with a half-purged directory and no idea which parts survived. + # Ownership is taken first, for the Administrators group via takeown /A and the + # well-known SID: neither the group name nor a takeown answer is the same on every + # Windows language. + $ownershipFailures = @(Grant-NodePilotPurgeAccess -Path $DataPath) + if ($ownershipFailures.Count -gt 0) { + # Nothing is deleted from a tree that is only partly accessible, so a retry starts from + # the same state. $dataPathRemaining = $true - Write-Warn " Could not fully delete $DataPath : $($_.Exception.Message)" - $survivors = @(Get-ChildItem -LiteralPath $DataPath -Recurse -Force -File -ErrorAction SilentlyContinue) - Write-Warn " $($survivors.Count) file(s) remain. Delete the folder by hand, or reboot and retry." + foreach ($failure in $ownershipFailures) { Write-Warn " $failure" } + Write-Warn " $DataPath was not deleted. Resolve the failures above and run the uninstall again." + } + else { + try { + Remove-Item -LiteralPath $DataPath -Recurse -Force -ErrorAction Stop + Write-Info " Deleted: $DataPath" + } + catch { + # Report and continue. Aborting here would skip the closing report, and the operator + # would be left with a half-purged directory and no idea which parts survived. + $dataPathRemaining = $true + Write-Warn " Could not fully delete $DataPath : $($_.Exception.Message)" + $survivors = @(Get-ChildItem -LiteralPath $DataPath -Recurse -Force -File -ErrorAction SilentlyContinue) + Write-Warn " $($survivors.Count) file(s) remain. Delete the folder by hand, or reboot and retry." + } } } else { Write-Info " Data path not present." @@ -401,7 +443,50 @@ if ($installedThumbprint -and $isManagedAccount) { Write-Host "" if ($installPathRemaining -or $backupsRemaining -or $dataPathRemaining) { - Write-Warn 'Uninstall finished with leftovers (see above).' + # The marker and deploy\ stay, so the setup uninstaller and this script can be run again. + Write-Warn 'Uninstall finished with leftovers (see above). The installation marker and the deploy scripts were kept; run the uninstall again once the cause is fixed.' exit 1 } + +Write-Step "Removing the deploy scripts and the installation marker" +$recoveryHelperPath = Join-Path $InstallPath 'deploy' +if (Test-Path -LiteralPath $recoveryHelperPath) { + try { Remove-Item -LiteralPath $recoveryHelperPath -Recurse -Force -ErrorAction Stop } + catch { + Write-Warn " Could not delete $recoveryHelperPath : $($_.Exception.Message)" + Write-Warn 'Uninstall finished with leftovers. The installation marker was kept; run the uninstall again.' + exit 1 + } + Write-Info " Deleted: $recoveryHelperPath" +} +# Only when nothing is left. Inno removes its own uninstaller and the empty directory after this +# script returns. +if (Test-Path -LiteralPath $InstallPath) { + $remaining = @(Get-ChildItem -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue) + if ($remaining.Count -eq 0) { + Remove-Item -LiteralPath $InstallPath -Force -ErrorAction SilentlyContinue + Write-Info " Deleted: $InstallPath" + } + else { + Write-Info ' Leaving the setup uninstaller to remove itself and the directory.' + } +} +# Without this, a fresh install after an uninstall is misdetected as an upgrade forever - the +# setup wizard reads this key to choose between the two. +$markerPath = 'HKLM:\SOFTWARE\NodePilot\Server' +if (Test-Path -LiteralPath $markerPath) { + Remove-Item -LiteralPath $markerPath -Recurse -Force -ErrorAction SilentlyContinue + Write-Info " Removed: $markerPath" +} else { + Write-Info " No installation marker present." +} +# Drop the parent only when this uninstall emptied it; something else may live under it. +$markerParent = 'HKLM:\SOFTWARE\NodePilot' +if ((Test-Path -LiteralPath $markerParent) -and + -not (Get-ChildItem -LiteralPath $markerParent -ErrorAction SilentlyContinue) -and + -not (Get-Item -LiteralPath $markerParent -ErrorAction SilentlyContinue).GetValueNames()) { + Remove-Item -LiteralPath $markerParent -Force -ErrorAction SilentlyContinue +} Write-Ok "Uninstall complete." +try { Stop-Transcript | Out-Null } catch { } +Remove-Item -LiteralPath $TranscriptPath -Force -ErrorAction SilentlyContinue diff --git a/deploy/server/Build-ServerInstaller.ps1 b/deploy/server/Build-ServerInstaller.ps1 index 4dfd3ac4..64cbae46 100644 --- a/deploy/server/Build-ServerInstaller.ps1 +++ b/deploy/server/Build-ServerInstaller.ps1 @@ -144,6 +144,7 @@ Write-Step 'Staging the deployment scripts' $deployScripts = @( 'ArtifactSecurity.ps1' 'DeploymentProgress.ps1' + 'LocalServerHttp.ps1' 'MachinePath.ps1' 'SwitcherConfig.ps1' 'Preflight.ps1' diff --git a/deploy/server/NodePilotServer.iss b/deploy/server/NodePilotServer.iss index 8d4845ec..abd0aadb 100644 --- a/deploy/server/NodePilotServer.iss +++ b/deploy/server/NodePilotServer.iss @@ -2234,11 +2234,16 @@ begin ScriptPath := ExpandConstant('{app}\deploy\Uninstall-NodePilot.ps1'); if not FileExists(ScriptPath) then begin + // The script removes the marker only after a complete cleanup, so without the marker there + // is nothing left for it to do and Inno may finish. + if not RegKeyExists(HKLM64, 'SOFTWARE\NodePilot\Server') then Exit; SuppressibleMsgBox('The NodePilot uninstall script is missing:' + #13#10 + ScriptPath + #13#10#13#10 + - 'The Windows service and its firewall rules have NOT been removed. Run ' + - 'Uninstall-NodePilot.ps1 by hand, or reinstall and uninstall again.', + 'The Windows service and its firewall rules have NOT been removed, and this uninstall ' + + 'was stopped. Reinstall NodePilot Server, then uninstall again.', mbCriticalError, MB_OK, IDOK); - Exit; + // Abort in usUninstall ends the uninstaller with a non-zero exit code before Inno removes + // any file or registry entry, so the installation stays retryable. + Abort; end; // Built HERE, at uninstall time. This is the whole reason the [UninstallRun] section is gone. @@ -2273,15 +2278,25 @@ begin Arguments := Arguments + Switches; if not Exec('powershell.exe', Arguments, '', SW_HIDE, ewWaitUntilTerminated, ResultCode) then - SuppressibleMsgBox('Could not start PowerShell to remove the NodePilot service.', - mbCriticalError, MB_OK, IDOK) + begin + SuppressibleMsgBox('Could not start PowerShell to remove the NodePilot service. ' + + 'Nothing was removed; run the uninstall again.', + mbCriticalError, MB_OK, IDOK); + Abort; + end else if ResultCode <> 0 then - // Reported, not swallowed. The script exits non-zero when it had to leave files behind, and - // an operator who is told that can act on it. - SuppressibleMsgBox('The NodePilot uninstall reported problems (exit code ' + + begin + // The script exits non-zero while something is left. It keeps the marker and deploy\ in + // that case, and the abort keeps the uninstaller and its Apps & Features entry, so the + // same uninstall can be repeated once the cause is fixed. + SuppressibleMsgBox('The NodePilot uninstall could not finish (exit code ' + IntToStr(ResultCode) + ').' + #13#10#13#10 + - 'Check %TEMP% for details; some files or the Windows service may still be present.', + 'The uninstall was stopped and NodePilot Server stays registered. Fix the problem ' + + 'named in the log, then uninstall again. The log is at:' + #13#10 + + GetEnv('TEMP') + '\nodepilot-server-uninstall.log', mbError, MB_OK, IDOK); + Abort; + end; end; if CurUninstallStep = usPostUninstall then diff --git a/src/nodepilot-docs-ui/content/de/deployment/production.md b/src/nodepilot-docs-ui/content/de/deployment/production.md index 4fef5126..dcc48c57 100644 --- a/src/nodepilot-docs-ui/content/de/deployment/production.md +++ b/src/nodepilot-docs-ui/content/de/deployment/production.md @@ -678,6 +678,11 @@ Nach einer Installation über das GUI-Setup geht dasselbe über „Apps & Featur & 'C:\Program Files\NodePilot\unins000.exe' /VERYSILENT /SUPPRESSMSGBOXES /PURGEDATA=1 ``` +Lässt sich etwas nicht entfernen, bricht die Deinstallation mit einem Exit-Code ungleich 0 ab und +behält den Installations-Marker, die Deploy-Skripte und den Eintrag unter „Apps & Features". Die +Ursache steht in `%TEMP%\nodepilot-server-uninstall.log`; sie beheben und dieselbe Deinstallation +erneut ausführen. + **Die Datenbank wird nie entfernt, und es gibt dafür keine Option.** NodePilot legt sie nicht an — sie wurde in Kapitel 2 separat bereitgestellt, hat oft ein eigenes Backup- und Replikationsregime, und in einem Active/Passive-Cluster teilen sich beide Knoten dieselbe. Aus diff --git a/src/nodepilot-docs-ui/content/en/deployment/production.md b/src/nodepilot-docs-ui/content/en/deployment/production.md index 8f9d929c..f76edd2c 100644 --- a/src/nodepilot-docs-ui/content/en/deployment/production.md +++ b/src/nodepilot-docs-ui/content/en/deployment/production.md @@ -676,6 +676,10 @@ After an installation through the GUI setup, the same is possible through "Apps & 'C:\Program Files\NodePilot\unins000.exe' /VERYSILENT /SUPPRESSMSGBOXES /PURGEDATA=1 ``` +If something cannot be removed, the uninstall stops with a non-zero exit code and keeps the +installation marker, the deploy scripts and the "Apps & features" entry. The cause is in +`%TEMP%\nodepilot-server-uninstall.log`; fix it and run the same uninstall again. + **The database is never removed, and there is no option for it.** NodePilot does not create it — it was provisioned separately in chapter 2, often has its own backup and replication regime, and in an active/passive cluster both nodes share it. For the same reason, the gMSA's "Log on as a service"