diff --git a/deploy/ArtifactSecurity.ps1 b/deploy/ArtifactSecurity.ps1 index 54a16d2..45eaa46 100644 --- a/deploy/ArtifactSecurity.ps1 +++ b/deploy/ArtifactSecurity.ps1 @@ -28,9 +28,37 @@ function New-NodePilotRandomBase64 { } function Import-NodePilotPkcsTypes { + <# + The assembly carrying SignedCms is named differently per edition: Windows PowerShell 5.1 + ships it inside System.Security, PowerShell 7 as System.Security.Cryptography.Pkcs. + + Asked by edition rather than attempted and caught. The try/catch this replaces worked, but + Add-Type raises a TERMINATING error under the setup's Stop preference, and Start-Transcript + records it before the catch can swallow it - so every setup log carried a red + "Die Assembly ... konnte nicht gefunden werden" immediately above the line confirming the + signature had verified. An operator reading that in CMTrace has every reason to abort a + perfectly healthy installation. + #> if ('System.Security.Cryptography.Pkcs.SignedCms' -as [type]) { return } - try { Add-Type -AssemblyName System.Security.Cryptography.Pkcs -ErrorAction Stop } - catch { Add-Type -AssemblyName System.Security -ErrorAction Stop } + if ($PSVersionTable.PSEdition -eq 'Core') { + Add-Type -AssemblyName System.Security.Cryptography.Pkcs -ErrorAction Stop + } + else { + Add-Type -AssemblyName System.Security -ErrorAction Stop + } +} + +function Import-NodePilotZipTypes { + <# + System.IO.Compression.ZipFile lives in a separate assembly that Windows PowerShell 5.1 does + not load on its own; PowerShell 7 has it in the default set. Asked by edition for the same + reason as Import-NodePilotPkcsTypes: a try/catch here would write a red terminating error + into every setup transcript before swallowing it. + #> + if ('System.IO.Compression.ZipFile' -as [type]) { return } + if ($PSVersionTable.PSEdition -ne 'Core') { + Add-Type -AssemblyName System.IO.Compression.FileSystem -ErrorAction Stop + } } function ConvertFrom-NodePilotHex { @@ -420,9 +448,35 @@ function Expand-NodePilotArtifactToStaging { [string]$ParentPath = [IO.Path]::GetTempPath() ) + # Absolute paths: ExtractToDirectory resolves relative ones against the PROCESS working + # directory, which is not the caller's location and is not the staging parent either. + $resolvedArtifact = (Resolve-Path -LiteralPath $ArtifactPath -ErrorAction Stop).Path $stagingPath = New-NodePilotRestrictedStagingDirectory -ParentPath $ParentPath try { - Expand-Archive -LiteralPath $ArtifactPath -DestinationPath $stagingPath -Force + # ExtractToDirectory rather than Expand-Archive, which pays per-entry pipeline overhead + # that dominates a tree of mostly sub-64 KB files. Faster everywhere measured, but the + # margin is strongly environment-dependent, so do not quote a single multiplier: + # + # Win 11 workstation (PS 5.1.22621) 24.3-24.8 s -> 2.0-2.7 s ~9-12x + # Server 2025 VM, 4 cores (26100) 7.3- 7.7 s -> 4.6-5.2 s ~1.6x + # + # Same artifact (2867 files, 114 MB), three and two runs respectively. The workstation is + # the faster machine on the new path and the slower one on the old, so the difference is + # in what Expand-Archive costs per entry there, not in raw I/O. Worth having on both, but + # the lab number is the one to expect on a server. + # + # The two were verified to produce identical trees: same 2867 files, same 377 directories, + # zero differences in relative path, length or SHA-256. + # + # Zip-slip protection is NOT lost in the trade. .NET refuses an entry whose resolved path + # leaves the destination ("Durch Extrahieren des Zip-Eintrags wuerde eine Datei ausserhalb + # des angegebenen Zielverzeichnisses erstellt"), verified against a crafted archive + # carrying '../escaped.txt'. Assert-NodePilotExtractedFiles below is the second line + # anyway: it rejects rooted or dot-dot paths in the manifest, requires an exact file + # count, and hashes every file against the signed manifest - so an entry that landed + # somewhere unexpected inside staging would fail there regardless. + Import-NodePilotZipTypes + [IO.Compression.ZipFile]::ExtractToDirectory($resolvedArtifact, $stagingPath) Assert-NodePilotExtractedFiles -RootPath $stagingPath return $stagingPath } diff --git a/deploy/SetupContract.ps1 b/deploy/SetupContract.ps1 index 6dbabcf..330e7bc 100644 --- a/deploy/SetupContract.ps1 +++ b/deploy/SetupContract.ps1 @@ -366,9 +366,13 @@ $script:NodePilotInstallPhases = @( [pscustomobject]@{ Step = 'Starting service'; Percent = 80; Text = 'Starting the service - this can take up to three minutes' } ) -# The updater's four phases. The probe here waits 60 s, not the installer's 180, so the last +# The updater's five phases. The probe here waits 60 s, not the installer's 180, so the last # caption promises less. $script:NodePilotUpdatePhases = @( + # Ahead of the backup on purpose: expanding ~2900 files to staging and hashing every one of + # them against the signed manifest is the longest part of an update, and it used to happen + # with no phase of its own - the dialog stood on its start caption until the backup began. + [pscustomobject]@{ Step = 'Extracting artifact'; Percent = 10; Text = 'Extracting and verifying the signed artifact - this can take a few minutes' } [pscustomobject]@{ Step = 'Backing up current install'; Percent = 20; Text = 'Backing up the current installation' } [pscustomobject]@{ Step = 'Stopping service'; Percent = 40; Text = 'Stopping the service' } [pscustomobject]@{ Step = 'Installing verified artifact'; Percent = 55; Text = 'Installing the verified artifact' } diff --git a/deploy/Test-ArtifactSecurity.ps1 b/deploy/Test-ArtifactSecurity.ps1 index 07ef6d0..4477d46 100644 --- a/deploy/Test-ArtifactSecurity.ps1 +++ b/deploy/Test-ArtifactSecurity.ps1 @@ -84,6 +84,36 @@ try { throw 'Extracted artifact tampering was not detected.' } + # Zip-slip. The extractor was swapped from Expand-Archive to ZipFile::ExtractToDirectory for + # speed (2.2 s against 25.8 s on the real artifact), and the whole reason that trade is + # acceptable is that .NET refuses an entry resolving outside the destination. Asserted here + # rather than assumed, because a future swap to a hand-rolled extraction loop would silently + # give it up: this is the one property the staging directory cannot recover from. + $slipZip = Join-Path $testRoot 'zip-slip.zip' + $slipSource = Join-Path $testRoot 'slip-src' + New-Item -ItemType Directory -Path $slipSource -Force | Out-Null + [IO.File]::WriteAllText((Join-Path $slipSource 'harmless.txt'), 'ok', (New-Object Text.UTF8Encoding($false))) + Compress-Archive -Path (Join-Path $slipSource '*') -DestinationPath $slipZip -Force + # Compress-Archive cannot author a traversing entry name, so rewrite one in directly. + Import-NodePilotZipTypes + $slipArchive = [IO.Compression.ZipFile]::Open($slipZip, [IO.Compression.ZipArchiveMode]::Update) + try { + $entry = $slipArchive.CreateEntry('../escaped.txt') + $writer = New-Object IO.StreamWriter($entry.Open()) + try { $writer.Write('escaped') } finally { $writer.Dispose() } + } + finally { $slipArchive.Dispose() } + + $slipBlocked = $false + try { [void](Expand-NodePilotArtifactToStaging -ArtifactPath $slipZip -ParentPath $testRoot) } + catch { $slipBlocked = $true } + if (-not $slipBlocked) { + throw 'A zip entry escaping the staging directory was extracted instead of rejected.' + } + if (Test-Path -LiteralPath (Join-Path $testRoot 'escaped.txt')) { + throw 'Zip-slip wrote a file outside the staging directory.' + } + $secretPath = Join-Path $testRoot 'restricted-settings.json' $secretBytes = [Text.Encoding]::UTF8.GetBytes('{"secret":"first"}') try { diff --git a/deploy/Test-SetupAdapter.ps1 b/deploy/Test-SetupAdapter.ps1 index 597d5e9..ea419d0 100644 --- a/deploy/Test-SetupAdapter.ps1 +++ b/deploy/Test-SetupAdapter.ps1 @@ -602,6 +602,7 @@ try { # Every phase either script announces has to be recognised - the reverse of the drift guard, # checked here against the real tables rather than against the scripts. foreach ($sample in @( + '[update] Extracting artifact', '[update] Backing up current install', "[update] Stopping service 'NodePilot'", '[update] Installing verified artifact', @@ -609,6 +610,16 @@ try { Assert-True -Name "the updater phase in '$sample' is recognised" ` -Condition ($null -ne (Get-NodePilotPhaseProgress -Line $sample)) } + # Extraction has to come FIRST, and it has to exist at all. It is the longest part of an + # update - ~2900 files expanded to staging, then hashed one by one against the signed manifest + # - and it used to run with no phase of its own, so the dialog sat on its start caption for + # minutes while an operator reasonably concluded the upgrade had hung. A future edit that + # renames the heading or reorders the table past the backup brings that back. + $extractPhase = Get-NodePilotPhaseProgress -Line '[update] Extracting artifact' + $backupPhase = Get-NodePilotPhaseProgress -Line '[update] Backing up current install' + Assert-True -Name 'the updater announces extraction before the backup' ` + -Condition ($null -ne $extractPhase -and $null -ne $backupPhase -and + [int]$extractPhase.Percent -lt [int]$backupPhase.Percent) Assert-True -Name 'an updater detail line is not a phase' ` -Condition ($null -eq (Get-NodePilotPhaseProgress -Line '[update] Backup: C:\x')) # Ascending percentages are what let the wizard refuse to ever move the bar backwards without diff --git a/deploy/Update-NodePilot.ps1 b/deploy/Update-NodePilot.ps1 index 01857cd..9855dfe 100644 --- a/deploy/Update-NodePilot.ps1 +++ b/deploy/Update-NodePilot.ps1 @@ -189,6 +189,14 @@ try { } # Reject a malformed signed ZIP before stopping the service or touching the installation. + # + # Announced, because this is the longest stretch of the whole update and used to run without a + # single progress line: the first phase below is the backup, so the wizard sat on its start + # caption while ~2900 files were expanded to staging and then hashed one by one against the + # signed manifest. Measured 2.7 s of hashing on a warm NVMe, minutes on a machine whose + # real-time scanner inspects every file created under %TEMP% - see docs/av-exclusions.md. + # An operator watching a motionless dialog reasonably concludes it has hung. + Write-Step 'Extracting artifact' $artifactStage = Expand-NodePilotArtifactToStaging -ArtifactPath $ArtifactPath Write-Info "Verified restricted staging: $artifactStage" diff --git a/deploy/server/NodePilotServer.iss b/deploy/server/NodePilotServer.iss index 1b1300e..27c1af1 100644 --- a/deploy/server/NodePilotServer.iss +++ b/deploy/server/NodePilotServer.iss @@ -53,12 +53,23 @@ MinVersion=10.0.20348 OutputDir={#OutputDir} OutputBaseFilename=NodePilot-Server-Setup-{#AppVersion} WizardStyle=modern -; Default window size. WizardSizePercent=120 was tried and reverted - every page fits at 100%, and -; the pages that are tight are tight by layout rather than by window size. +; Bigger than default, measured rather than guessed. An earlier note here claimed "every page fits +; at 100%" and reverted a previous attempt on that basis; the prerequisites page disproves it once +; all ten checks report. Each row is "Title: Detail" (see the render loop), so at the default width +; six of ten wrap to two lines, LayoutReadiness stacks ~238 px of rows into a ~309 px surface, and +; the remediation box - which gets whatever is left - lands at ~30 px. That is one clipped line for +; the field that has to show a CREATE LOGIN / CREATE USER block when the database check fails. ; -; WizardResizable stays off regardless of that: the controls on the network and prerequisites pages -; are positioned once, at wizard construction, and carry no anchors. A window the operator drags -; open would grow around a certificate picker that stays where it was. +; Width does most of the work: +25% (497 -> ~621 px) puts most rows back on one line, which shortens +; the stack before height is even considered. Height then buys the explanation real room: +45% +; (360 -> ~522 px, surface ~309 -> ~471 px) leaves ~210 px, about twelve lines. The window is +; ~560 px tall including its frame and fits a 768 px server console. +; +; WizardResizable stays off regardless: the controls on the network and prerequisites pages are +; positioned once, at wizard construction, and carry no anchors. A window the operator drags open +; would grow around a certificate picker that stays where it was. A fixed larger START size is a +; different thing and is safe - every control that must grow is already sized from SurfaceWidth. +WizardSizePercent=125,145 SetupIconFile={#StageDir}\setup-icon.ico LicenseFile={#StageDir}\LICENSE.txt ; A failed setup has to be diagnosable without asking the operator to reproduce it. @@ -614,7 +625,15 @@ begin Available := ButtonTop - ScaleY(6) - RemediationBox.Top; // Never negative, never overlapping the buttons: with every row wrapped and every fix offered // there is little left, and a label with no room is still better than one drawn over them. - if Available < ScaleY(13) then Available := ScaleY(13); + // + // The floor is two lines plus the scrollbar rather than one. At one line the control reads as a + // broken edit field instead of an explanation - which is exactly what it looked like with ten + // checks before the window was enlarged. The larger window means this is not reached in + // practice; it is here for high-DPI scaling and for the day an eleventh check arrives. + // + // Rows still win and the box still gives: the other precedence would draw the explanation over + // the last checks, and a check nobody can see is worse than an explanation that has to scroll. + if Available < ScaleY(34) then Available := ScaleY(34); RemediationBox.Height := Available; end; diff --git a/docs/av-exclusions.md b/docs/av-exclusions.md index 6583408..952e95e 100644 --- a/docs/av-exclusions.md +++ b/docs/av-exclusions.md @@ -63,6 +63,7 @@ Rolle: Windows-Server mit dem Dienst **`NodePilot`** (Anzeigename `NodePilot Orc | `C:\Program Files\NodePilot\` | Programmverzeichnis: `NodePilot.Api.exe`, ~mehrere hundert verwaltete DLLs, `wwwroot\` (SPA), `PSModules\`, `knowledge\` | Wird beim Update komplett getauscht; ein gehaltenes Scanner-Handle lässt Verschieben/Löschen fehlschlagen. Enthält außerdem `powershell.config.json` — siehe Warnung unter [A.4](#a4-verhaltensregeln-asr-controlled-folder-access) | Pflicht | Schreibrechte hat nur SYSTEM/Administratoren; ein Angreifer mit diesen Rechten hat das System ohnehin. Restrisiko: eine dort abgelegte Fremd-DLL würde nicht mehr gescannt — kompensierbar über Publisher-Regel statt Pfad und über Integritätsüberwachung des Verzeichnisses | | `C:\ProgramData\NodePilot\` | Laufzeitdaten: `logs\`, `archive\`, `jwt-secret.key`, `data-protection-keys\`, `admin-setup.token`, `appsettings.runtime.json`, `install-report.txt`, `postgres-root-ca.pem` | Dauerhaftes Schreiben (Rolling Logs, atomare Config-Writes über `.tmp` + `File.Replace`, gzip-Archive). Scanner-Handles auf der Zieldatei lassen den atomaren Ersetzungsschritt scheitern | Pflicht | Verzeichnis ist per ACL auf das Dienstkonto + Administratoren beschränkt. Es liegen dort **Schlüsselmaterial und Tokens** — der Ausschluss verhindert nicht deren Diebstahl (dafür ist der Dateizugriff zuständig), er reduziert nur die Erkennung einer dort abgelegten Schaddatei | | `C:\Program Files\NodePilot.rollback.*`
`C:\Program Files\NodePilot.backup.*` | Zeitgestempelte Kopien des vorherigen Programmverzeichnisses (drei werden aufbewahrt) | Entstehen nur während Installation/Update; enthalten dieselben Binärdateien wie oben | Empfohlen | Wie Programmverzeichnis. Kann auf ein Wartungsfenster befristet werden | +| `%TEMP%\nodepilot-artifact-*`
(Dienst-Kontext: `C:\Windows\Temp\nodepilot-artifact-*`) | Staging des signierten Artefakts: Installer **und** Updater entpacken das Zip zuerst hierher — rund **2900 Dateien**, davon ~2650 kleiner als 64 KB, und prüfen anschließend jede einzelne gegen das signierte Manifest | Dies ist die **teuerste Stelle des gesamten Updates** und der Grund, warum ein Upgrade minutenlang auf einer Stelle zu stehen scheint: Nicht die Datenmenge (114 MB) kostet, sondern die Dateianzahl. Ein Echtzeit-Scan prüft jede Erzeugung einzeln und vervielfacht die Laufzeit; ein gehaltenes Handle lässt zusätzlich das Aufräumen des Staging-Ordners scheitern | Empfohlen | Der Ordner trägt bereits eine restriktive DACL (SYSTEM + Administratoren + aufrufender Benutzer, atomar bei der Erzeugung gesetzt), und sein Inhalt wird unmittelbar nach dem Entpacken **gegen das signierte Manifest** verifiziert — Datei für Datei, mit Längen- und SHA-256-Vergleich. Der Ausschluss senkt die Erkennung also genau dort, wo bereits kryptografisch geprüft wird. Kann auf ein Wartungsfenster befristet werden | Ausdrücklich **nicht** enthalten: Ordner, in die Workflows schreiben. Siehe [Was nicht ausgeschlossen werden soll](#was-ausdrücklich-nicht-ausgeschlossen-werden-soll). diff --git a/src/nodepilot-docs-ui/content/deployment/av-exclusions.md b/src/nodepilot-docs-ui/content/deployment/av-exclusions.md index bb8bb65..a054023 100644 --- a/src/nodepilot-docs-ui/content/deployment/av-exclusions.md +++ b/src/nodepilot-docs-ui/content/deployment/av-exclusions.md @@ -32,6 +32,7 @@ Ein Dienst: `NodePilot` (Anzeigename `NodePilot Orchestrator`), ausgeführt als | `C:\Program Files\NodePilot` | Programmverzeichnis, wird beim Update vollständig getauscht | Pflicht | Nur SYSTEM und Administratoren haben Schreibrechte. Publisher-Regel statt Pfad bevorzugen | | `C:\ProgramData\NodePilot` | Logs, Archive, Schlüsselmaterial, Laufzeitkonfiguration | Pflicht | Enthält Schlüssel und Tokens; deren Schutz liegt bei der ACL, nicht beim Scanner | | `C:\Program Files\NodePilot.rollback.*`, `…NodePilot.backup.*` | Zeitgestempelte Vorgängerstände (drei werden aufbewahrt) | Empfohlen | Auf das Wartungsfenster befristbar | +| `%TEMP%\nodepilot-artifact-*` (Dienst: `C:\Windows\Temp\…`) | Staging des signierten Artefakts — ~2900 Dateien, davon ~2650 unter 64 KB. Die **teuerste Stelle eines Updates**: nicht die 114 MB kosten, sondern die Dateianzahl, und ein Echtzeit-Scan prüft jede Erzeugung einzeln | Empfohlen | Restriktive DACL bei der Erzeugung; der Inhalt wird direkt danach Datei für Datei gegen das signierte Manifest geprüft. Auf das Wartungsfenster befristbar | ### Prozesse