From c1e0b5474adda8b7dd1b89d2847da3373bb3b1f3 Mon Sep 17 00:00:00 2001 From: TheAbider <51920546+TheAbider@users.noreply.github.com> Date: Wed, 1 Jul 2026 09:39:17 -0700 Subject: [PATCH] Final audit sweep: firewall fail-open + iSCSI/netdiag/session fixes (v1.121.15) Fixes from the final consolidated audit of the remaining modules. 15-RDP.ps1: - RDP/WinRM 'restrict to subnet' now ALSO disables RackStack's own custom RDP ('RDP Inbound TCP/UDP 3389') and WinRM ('WinRM HTTP/HTTPS Inbound') firewall rules, which have no DisplayGroup and RemoteAddress=Any. The DisplayGroup-only disable missed them, so 3389/5985/5986 stayed reachable from ANY address while the tool reported the host as subnet-locked -- a silent security fail-open. - PowerShell-Remoting reports each firewall group's ACTUAL enabled state instead of a hardcoded green list (the Enable calls are -EA SilentlyContinue). 10-iSCSI.ps1: - The [M] Manual option in Connect-to-iSCSI-Targets is matched before the nav check (Test-NavigationCommand consumed 'M' as home and exited the menu -- 2nd instance of the class). - iSCSI Auto-Claim status reads the iSCSI key of the settings hashtable instead of the hashtable's (always-true) truthiness. - The connect dry-run undo matches the target portal via the session's connection (TargetAddress), not InitiatorPortalAddress (the local NIC IP) which matched nothing -- the undo now actually disconnects. 58-NetworkDiagnostics.ps1: - Throughput benchmark uses a case-distinct Mbps variable; $writeMbps collided with $writeMBps (PS is case-insensitive), so the MB/s column showed the 8x Mbps. 55-QoLFeatures.ps1: - The session-state file that Restore-SessionState reads is now written atomically (tmp -> rename), matching the snapshot file. Tests: Section 202 (10 asserts). Structural 5366/5366; Pester 312/312; PSSA 0. This completes an end-to-end adversarial audit of the whole codebase. Version -> 1.121.15. --- Changelog.md | 14 +++++++++ Header.ps1 | 2 +- Modules/00-Initialization.ps1 | 2 +- Modules/10-iSCSI.ps1 | 30 ++++++++++++++----- Modules/15-RDP.ps1 | 50 +++++++++++++++++++++++++------ Modules/55-QoLFeatures.ps1 | 9 +++++- Modules/58-NetworkDiagnostics.ps1 | 11 ++++--- README.md | 2 +- RackStack.ps1 | 2 +- RackStack.psd1 | 2 +- Tests/Run-Tests.ps1 | 43 +++++++++++++++++++++++++- 11 files changed, 140 insertions(+), 27 deletions(-) diff --git a/Changelog.md b/Changelog.md index 2798764..bf6309d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,19 @@ # Changelog +## v1.121.15 + +Final audit sweep — remaining fixes across the firewall/RDP, iSCSI, network-diagnostics, and session modules. + +- **"Restrict RDP to a subnet" now actually restricts it.** If you had previously added RDP (or WinRM) firewall rules via the Firewall Templates, the subnet-restriction step left those wide-open rules enabled — so RDP/WinRM stayed reachable from any address while the tool reported it was locked down. It now disables those rules too, so the restriction holds. +- **PowerShell Remoting reports its real firewall state.** It used to list every firewall group as "enabled" regardless; it now shows each group's actual status. +- **iSCSI fixes:** the [M] Manual option in "Connect to iSCSI Targets" works (it was silently exiting the menu); the status screen shows the real iSCSI auto-claim state instead of always "Enabled"; and the dry-run undo for a connection now actually disconnects. +- **The network throughput benchmark shows the correct MB/s.** A variable-name clash made the MB/s figure display the Mbps value (8× too high). +- **Session state is saved atomically**, so a crash mid-save can't corrupt the file the tool reads on resume. + +(This completes an end-to-end adversarial audit of the whole codebase; every remaining finding is fixed and verified.) + +No module or CLI action changes (81 modules, 201 actions). + ## v1.121.14 VM and storage safety — fixes found by a deep audit of the VM export/import, offline-VHD, VHD conversion, and Hyper-V Replica modules. diff --git a/Header.ps1 b/Header.ps1 index 61acc1a..d332e93 100644 --- a/Header.ps1 +++ b/Header.ps1 @@ -30,7 +30,7 @@ 7h3 4b1d3r .VERSION - 1.121.14 + 1.121.15 .LAST UPDATED 07/01/2026 diff --git a/Modules/00-Initialization.ps1 b/Modules/00-Initialization.ps1 index 58a22ea..abfe8a9 100644 --- a/Modules/00-Initialization.ps1 +++ b/Modules/00-Initialization.ps1 @@ -233,7 +233,7 @@ if (-not $PSCommandPath -and $script:ScriptPath) { if (-not $script:ModuleRoot -and $script:ScriptPath) { $script:ModuleRoot = [System.IO.Path]::GetDirectoryName($script:ScriptPath) } -$script:ScriptVersion = "1.121.14" +$script:ScriptVersion = "1.121.15" $script:ScriptStartTime = Get-Date # Post-update cleanup: UpdateSelf / Rollback leave a `.pending-delete` sibling next to RackStack.exe. diff --git a/Modules/10-iSCSI.ps1 b/Modules/10-iSCSI.ps1 index fe2aafe..e1bedba 100644 --- a/Modules/10-iSCSI.ps1 +++ b/Modules/10-iSCSI.ps1 @@ -1032,8 +1032,13 @@ function Connect-iSCSITargets { # portals. Doesn't unregister the portals themselves so a # subsequent re-connect is fast. foreach ($p in $capPortals) { - Get-IscsiSession -ErrorAction SilentlyContinue | Where-Object { $_.TargetNodeAddress -and $_.InitiatorPortalAddress -eq $p } | - ForEach-Object { Disconnect-IscsiTarget -NodeAddress $_.TargetNodeAddress -Confirm:$false -ErrorAction SilentlyContinue } + # Match on the session's CONNECTION target address (the target/SAN portal IP), + # not InitiatorPortalAddress — that's the LOCAL initiator NIC IP and never equals + # a captured target portal, so the old predicate matched nothing and the undo + # silently disconnected nothing. + Get-IscsiSession -ErrorAction SilentlyContinue | Where-Object { + $_.TargetNodeAddress -and (@($_ | Get-IscsiConnection -ErrorAction SilentlyContinue | Where-Object { $_.TargetAddress -eq $p }).Count -gt 0) + } | ForEach-Object { Disconnect-IscsiTarget -NodeAddress $_.TargetNodeAddress -Confirm:$false -ErrorAction SilentlyContinue } } }.GetNewClosure() Write-OutputColor " Queued (Dry-Run): connect iSCSI portals." -color "Warning" @@ -1312,11 +1317,16 @@ function Show-iSCSIStatus { Write-OutputColor " │$(" Load Balance Policy: Unknown".PadRight(72))│" -color "Warning" } - # Show MPIO disks - $mpioDisks = Get-MSDSMAutomaticClaimSettings -ErrorAction SilentlyContinue - if ($mpioDisks) { + # Show MPIO iSCSI auto-claim state. Get-MSDSMAutomaticClaimSettings returns a per-bus-type + # hashtable (e.g. @{ SAS = $true; iSCSI = $false }); a hashtable is ALWAYS truthy, so the old + # `if ($mpioDisks)` printed "Enabled" even when iSCSI auto-claim was off. Read the iSCSI key. + $mpioClaim = Get-MSDSMAutomaticClaimSettings -ErrorAction SilentlyContinue + if ($null -ne $mpioClaim -and $mpioClaim['iSCSI']) { Write-OutputColor " │$(" iSCSI Auto-Claim: Enabled".PadRight(72))│" -color "Success" } + else { + Write-OutputColor " │$(" iSCSI Auto-Claim: Disabled".PadRight(72))│" -color "Warning" + } } else { Write-OutputColor " │$(" MPIO: Not Installed".PadRight(72))│" -color "Warning" @@ -1556,8 +1566,14 @@ function Start-Show-iSCSISANMenu { Write-OutputColor "" -color "Info" $connectChoice = Read-Host " Select" - $navResult = Test-NavigationCommand -UserInput $connectChoice - if ($navResult.ShouldReturn) { return } + # Match this menu's own action letters (A/M/P) BEFORE the nav check — otherwise + # Test-NavigationCommand consumes 'M' as the home token and silently exits the + # whole iSCSI/SAN menu, making [M] Manual unreachable dead code (same class fixed + # in Set-iSCSIConfiguration). Only OTHER input (b/back/0/...) is treated as nav. + if ($connectChoice -notmatch '^[AaMmPp]$') { + $navResult = Test-NavigationCommand -UserInput $connectChoice + if ($navResult.ShouldReturn) { return } + } switch -Regex ($connectChoice) { '^[Aa]$' { diff --git a/Modules/15-RDP.ps1 b/Modules/15-RDP.ps1 index 223ce0e..e91b0ac 100644 --- a/Modules/15-RDP.ps1 +++ b/Modules/15-RDP.ps1 @@ -153,10 +153,17 @@ function Enable-RDP { -Profile Domain,Private,Public ` -Description "Allow RDP only from $subnet" -ErrorAction Stop | Out-Null - # Disable the broad RDP rules so only the restricted one applies + # Disable the broad RDP rules so only the restricted one applies. This must ALSO + # cover RackStack's own custom RDP rules ('RDP Inbound TCP/UDP 3389' from the + # Firewall Templates), which are created with NO DisplayGroup and RemoteAddress=Any + # — the DisplayGroup query below misses them, so they'd stay enabled and (Windows + # OR-combines inbound Allow rules) keep 3389 reachable from ANY address, silently + # defeating the subnet restriction this path claims to apply. Get-NetFirewallRule -DisplayGroup "Remote Desktop" -ErrorAction SilentlyContinue | Where-Object { $_.DisplayName -ne "RDP Subnet Restriction" } | Disable-NetFirewallRule -ErrorAction SilentlyContinue + Get-NetFirewallRule -DisplayName "RDP Inbound TCP 3389", "RDP Inbound UDP 3389" -ErrorAction SilentlyContinue | + Disable-NetFirewallRule -ErrorAction SilentlyContinue Write-OutputColor " RDP restricted to $subnet only." -color "Success" Write-OutputColor " Broad RDP rules disabled. Only subnet rule is active." -color "Success" @@ -164,6 +171,8 @@ function Enable-RDP { Add-UndoAction -Category "Security" -Description "RDP subnet restriction to $subnet" -UndoScript { Remove-NetFirewallRule -DisplayName "RDP Subnet Restriction" -ErrorAction SilentlyContinue Enable-NetFirewallRule -DisplayGroup "Remote Desktop" -ErrorAction SilentlyContinue + Get-NetFirewallRule -DisplayName "RDP Inbound TCP 3389", "RDP Inbound UDP 3389" -ErrorAction SilentlyContinue | + Enable-NetFirewallRule -ErrorAction SilentlyContinue } } catch { @@ -479,13 +488,30 @@ function Enable-PowerShellRemoting { Write-OutputColor "" -color "Info" Write-OutputColor " PowerShell Remoting enabled successfully!" -color "Success" Write-OutputColor "" -color "Info" - Write-OutputColor " Firewall rules enabled:" -color "Info" - Write-OutputColor " - Windows Remote Management (WinRM)" -color "Success" - Write-OutputColor " - Remote Event Log Management" -color "Success" - Write-OutputColor " - Remote Service Management" -color "Success" - Write-OutputColor " - Windows Management Instrumentation (WMI)" -color "Success" - Write-OutputColor " - Remote Scheduled Tasks Management" -color "Success" - Write-OutputColor " - File and Printer Sharing" -color "Success" + Write-OutputColor " Firewall rule groups:" -color "Info" + # Report each group's ACTUAL state rather than a hardcoded green list. The Enable calls above + # use -EA SilentlyContinue, so a group that is absent or failed to enable was previously still + # reported as enabled — false assurance about the remote-management firewall posture. + $rmGroups = @( + "Windows Remote Management", + "Remote Event Log Management", + "Remote Service Management", + "Windows Management Instrumentation (WMI)", + "Remote Scheduled Tasks Management", + "File and Printer Sharing" + ) + foreach ($grp in $rmGroups) { + $grpRules = @(Get-NetFirewallRule -DisplayGroup $grp -ErrorAction SilentlyContinue) + if (@($grpRules | Where-Object { $_.Enabled -eq 'True' }).Count -gt 0) { + Write-OutputColor " - $grp" -color "Success" + } + elseif ($grpRules.Count -eq 0) { + Write-OutputColor " - $grp (no rules present on this host)" -color "Warning" + } + else { + Write-OutputColor " - $grp (could NOT be enabled)" -color "Warning" + } + } Write-OutputColor "" -color "Info" Write-OutputColor " You can now connect to this server using:" -color "Info" Write-OutputColor " Enter-PSSession -ComputerName $env:COMPUTERNAME" -color "Success" @@ -530,15 +556,21 @@ function Enable-PowerShellRemoting { -Profile Domain,Private,Public ` -Description "Allow WinRM only from $subnet" -ErrorAction Stop | Out-Null - # Disable broad WinRM rules + # Disable broad WinRM rules — including RackStack's own custom 'WinRM HTTP/HTTPS + # Inbound' rules (Firewall Templates), which have no DisplayGroup + RemoteAddress=Any + # and would otherwise keep 5985/5986 open to any source, defeating the restriction. Get-NetFirewallRule -DisplayGroup "Windows Remote Management" -ErrorAction SilentlyContinue | Disable-NetFirewallRule -ErrorAction SilentlyContinue + Get-NetFirewallRule -DisplayName "WinRM HTTP Inbound", "WinRM HTTPS Inbound" -ErrorAction SilentlyContinue | + Disable-NetFirewallRule -ErrorAction SilentlyContinue Write-OutputColor " WinRM restricted to $subnet." -color "Success" Add-SessionChange -Category "Security" -Description "Restricted WinRM to subnet $subnet" Add-UndoAction -Category "Security" -Description "WinRM subnet restriction to $subnet" -UndoScript { Remove-NetFirewallRule -DisplayName "WinRM Subnet Restriction" -ErrorAction SilentlyContinue Enable-NetFirewallRule -DisplayGroup "Windows Remote Management" -ErrorAction SilentlyContinue + Get-NetFirewallRule -DisplayName "WinRM HTTP Inbound", "WinRM HTTPS Inbound" -ErrorAction SilentlyContinue | + Enable-NetFirewallRule -ErrorAction SilentlyContinue } } catch { diff --git a/Modules/55-QoLFeatures.ps1 b/Modules/55-QoLFeatures.ps1 index e7e3ba0..6d55abd 100644 --- a/Modules/55-QoLFeatures.ps1 +++ b/Modules/55-QoLFeatures.ps1 @@ -358,7 +358,14 @@ function Save-SessionState { } try { - $sessionState | ConvertTo-Json -Depth 10 | Out-File -LiteralPath $script:SessionStatePath -Encoding UTF8 -Force + # Atomic write — THIS is the file Restore-SessionState reads, and its load path deletes the + # file on a ConvertFrom-Json failure, so a mid-write truncation (process kill / disk full) + # would lose session state. Write to .tmp then rename so the reader only ever sees a complete + # file. (The snapshot file below already does this; the consumed file was missing it.) + $sessionJson = $sessionState | ConvertTo-Json -Depth 10 + $tmpSession = "$($script:SessionStatePath).tmp" + Set-Content -LiteralPath $tmpSession -Value $sessionJson -Encoding UTF8 -ErrorAction Stop + Move-Item -LiteralPath $tmpSession -Destination $script:SessionStatePath -Force -ErrorAction Stop } catch { Write-OutputColor " Warning: Could not save session state: $($_.Exception.Message)" -color "Warning" diff --git a/Modules/58-NetworkDiagnostics.ps1 b/Modules/58-NetworkDiagnostics.ps1 index 00cecee..33a6976 100644 --- a/Modules/58-NetworkDiagnostics.ps1 +++ b/Modules/58-NetworkDiagnostics.ps1 @@ -1179,8 +1179,11 @@ function Invoke-NetworkThroughputBenchmark { } if ($ok) { - $writeMbps = [math]::Round($writeMBps * 8, 0) - $readMbps = [math]::Round($readMBps * 8, 0) + # NOTE: PowerShell variable names are case-INSENSITIVE, so $writeMbps and $writeMBps are the + # SAME variable — the old code overwrote the MB/s value with the *8 Mbps value, then printed + # both columns from it (MB/s shown 8x inflated). Use case-distinct names for the Mbps values. + $writeMbits = [math]::Round($writeMBps * 8, 0) + $readMbits = [math]::Round($readMBps * 8, 0) $title = " THROUGHPUT: $target" if ($title.Length -gt 69) { $title = $title.Substring(0, 69) + "..." } Write-OutputColor "" -color "Info" @@ -1188,8 +1191,8 @@ function Invoke-NetworkThroughputBenchmark { Write-OutputColor " │$($title.PadRight(72))│" -color "Info" Write-OutputColor " ├────────────────────────────────────────────────────────────────────────┤" -color "Info" Write-OutputColor " │$(" Test size: ${sizeMB} MB".PadRight(72))│" -color "Info" - Write-OutputColor " │$(" Write: ${writeMBps} MB/s (${writeMbps} Mbps)".PadRight(72))│" -color "Success" - Write-OutputColor " │$(" Read: ${readMBps} MB/s (${readMbps} Mbps)".PadRight(72))│" -color "Success" + Write-OutputColor " │$(" Write: ${writeMBps} MB/s (${writeMbits} Mbps)".PadRight(72))│" -color "Success" + Write-OutputColor " │$(" Read: ${readMBps} MB/s (${readMbits} Mbps)".PadRight(72))│" -color "Success" Write-OutputColor " └────────────────────────────────────────────────────────────────────────┘" -color "Info" } Write-PressEnter diff --git a/README.md b/README.md index 92ad99c..5b08733 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ OpenSSF Best Practices codecov PSScriptAnalyzer 0 errors - 5357 structural tests + 5366 structural tests Pester 312 tests SLSA Level 3

diff --git a/RackStack.ps1 b/RackStack.ps1 index f2cb495..af64ecd 100644 --- a/RackStack.ps1 +++ b/RackStack.ps1 @@ -13,7 +13,7 @@ Environment-specific settings are configured via defaults.json. .VERSION - 1.121.14 + 1.121.15 .NOTES - Requires Windows Server 2012 R2 or later (or Windows 10/11 for testing) - Must be run as Administrator diff --git a/RackStack.psd1 b/RackStack.psd1 index 02080a0..f38cc11 100644 --- a/RackStack.psd1 +++ b/RackStack.psd1 @@ -1,6 +1,6 @@ @{ RootModule = 'RackStack.psm1' - ModuleVersion = '1.121.14' + ModuleVersion = '1.121.15' GUID = 'c19b8e71-4a35-4f2b-9d06-8a24f7bc0e91' Author = 'TheAbider' CompanyName = 'TheAbider' diff --git a/Tests/Run-Tests.ps1 b/Tests/Run-Tests.ps1 index 0cd87ce..598b294 100644 --- a/Tests/Run-Tests.ps1 +++ b/Tests/Run-Tests.ps1 @@ -1,6 +1,6 @@ <# .SYNOPSIS - Automated Test Runner for RackStack v1.121.14 + Automated Test Runner for RackStack v1.121.15 .DESCRIPTION Comprehensive non-interactive test suite covering: @@ -10131,6 +10131,47 @@ catch { Write-TestResult "VM-Lifecycle + Storage Hardening Tests" $false $_.Exception.Message } +# ============================================================================ +# SECTION 202: FINAL-SWEEP HARDENING (v1.121.15) +# ============================================================================ +# Guards the fixes from the final consolidated audit (Firewall/RDP, iSCSI, NetDiag, QoL): +# [0] RDP subnet restriction also disables the tool's own custom (no-group, any-address) RDP rules. +# [1] WinRM subnet restriction likewise disables the custom WinRM rules. +# [2] The [M] Manual option in Connect-to-iSCSI-Targets is matched before the nav check. +# [3] iSCSI Auto-Claim status reads the iSCSI key (was truthiness of the hashtable). +# [4] Throughput benchmark uses case-distinct Mbps var (was overwriting MB/s -> 8x inflated). +# [5] PowerShell-Remoting firewall groups are reported by actual state, not a hardcoded list. +# [6] iSCSI connect dry-run undo matches the target-portal via the connection (not initiator IP). +# [7] Session-state save writes the CONSUMED file atomically (tmp -> rename). +Write-SectionHeader "SECTION 202: FINAL-SWEEP HARDENING" + +try { + $rdpRaw = Get-Content "$modulesPath\15-RDP.ps1" -Raw + $iscRaw = Get-Content "$modulesPath\10-iSCSI.ps1" -Raw + $ndRaw = Get-Content "$modulesPath\58-NetworkDiagnostics.ps1" -Raw + $qolRaw = Get-Content "$modulesPath\55-QoLFeatures.ps1" -Raw + + # [0]/[1] subnet restriction disables the custom no-group allow rules + Write-TestResult "15-RDP: subnet restrict disables custom RDP rules [0]" ($rdpRaw -match 'Get-NetFirewallRule -DisplayName "RDP Inbound TCP 3389", "RDP Inbound UDP 3389" -ErrorAction SilentlyContinue \|[\s\S]{0,80}Disable-NetFirewallRule') + Write-TestResult "15-RDP: subnet restrict disables custom WinRM rules [1]" ($rdpRaw -match 'Get-NetFirewallRule -DisplayName "WinRM HTTP Inbound", "WinRM HTTPS Inbound" -ErrorAction SilentlyContinue \|[\s\S]{0,80}Disable-NetFirewallRule') + # [2] + Write-TestResult "10-iSCSI: Connect [M] matched before nav check [2]" ($iscRaw -match "connectChoice -notmatch '\^\[AaMmPp\]\`$'") + # [3] + Write-TestResult "10-iSCSI: Auto-Claim reads the iSCSI key [3]" ($iscRaw -match "\`$mpioClaim\['iSCSI'\]") + # [4] + Write-TestResult "58-NetDiag: throughput uses case-distinct Mbps var [4]" (($ndRaw -match '\$writeMbits = \[math\]::Round\(\$writeMBps \* 8') -and ($ndRaw -cmatch '\$\{writeMbits\} Mbps')) + Write-TestResult "58-NetDiag: no case-colliding Mbps display var [4]" (-not ($ndRaw -cmatch '\$\{writeMbps\}')) + # [5] + Write-TestResult "15-RDP: PS-remoting firewall groups reported by real state [5]" ($rdpRaw -match '\$rmGroups = @\([\s\S]{0,300}foreach \(\$grp in \$rmGroups\)') + # [6] + Write-TestResult "10-iSCSI: connect undo matches target portal via connection [6]" ($iscRaw -match 'Get-IscsiConnection -ErrorAction SilentlyContinue \| Where-Object \{ \$_\.TargetAddress -eq \$p \}') + # [7] + Write-TestResult "55-QoL: consumed session-state file written atomically [7]" ($qolRaw -match '\$tmpSession = "\$\(\$script:SessionStatePath\)\.tmp"[\s\S]{0,200}Move-Item -LiteralPath \$tmpSession') +} +catch { + Write-TestResult "Final-Sweep Hardening Tests" $false $_.Exception.Message +} + # ============================================================================ # SECTION 174: DOCUMENTATION FRESHNESS (counts must match the codebase) # ============================================================================