From aa62b66fd8a74b0a912aee16988ef0cab35cc065 Mon Sep 17 00:00:00 2001 From: Lord Hepipud Date: Tue, 23 Dec 2025 17:25:23 +0100 Subject: [PATCH] Fixes security catalog compilation on non-english Windows versions --- doc/100-General/10-Changelog.md | 1 + .../Set-IcingaAgentServicePermission.psm1 | 33 +---- .../Test-IcingaAgentServicePermission.psm1 | 6 + .../windows/Uninstall-IcingaServiceUser.psm1 | 11 +- .../Update-IcingaWindowsUserPermission.psm1 | 117 +++++++++++++++--- 5 files changed, 115 insertions(+), 53 deletions(-) diff --git a/doc/100-General/10-Changelog.md b/doc/100-General/10-Changelog.md index b5e9836..790085e 100644 --- a/doc/100-General/10-Changelog.md +++ b/doc/100-General/10-Changelog.md @@ -14,6 +14,7 @@ Released closed milestones can be found on [GitHub](https://github.com/Icinga/ic ### Bugfixes * [#833](https://github.com/Icinga/icinga-powershell-framework/issues/833) Fixes registry lookup for Icinga Agent installation to check if the required `DisplayName` attribute is defined before checking +* [#834](https://github.com/Icinga/icinga-powershell-framework/issues/834) Fixes security catalog compilation error on non-english Windows versions, while properly skipping checks on system SID's and improves security by always adding the `SeDenyNetworkLogonRight` and `SeDenyInteractiveLogonRight` privilege section for the JEA user SID * [#835](https://github.com/Icinga/icinga-powershell-framework/pull/835) Fixes JEA compiler to always enforce a rebuild of the Framework to ensure integrity of JEA profiles * [#836](https://github.com/Icinga/icinga-powershell-framework/issues/836) Fixes Metric over Time collector not working on Windows 2012 R2 and older diff --git a/lib/core/icingaagent/setters/Set-IcingaAgentServicePermission.psm1 b/lib/core/icingaagent/setters/Set-IcingaAgentServicePermission.psm1 index f3e1033..bb994d3 100644 --- a/lib/core/icingaagent/setters/Set-IcingaAgentServicePermission.psm1 +++ b/lib/core/icingaagent/setters/Set-IcingaAgentServicePermission.psm1 @@ -5,42 +5,15 @@ function Set-IcingaAgentServicePermission() return; } - $SystemPermissions = New-IcingaTemporaryFile; - $ServiceUser = Get-IcingaServiceUser; - $ServiceUserSID = Get-IcingaUserSID $ServiceUser; - $SystemContent = Get-IcingaAgentServicePermission; - $NewSystemContent = @(); + $ServiceUser = Get-IcingaServiceUser; + $ServiceUserSID = Get-IcingaUserSID $ServiceUser; if ([string]::IsNullOrEmpty($ServiceUser)) { Write-IcingaTestOutput -Severity 'Failed' -Message 'There is no user assigned to the Icinga 2 service or the service is not yet installed'; return $FALSE; } - foreach ($line in $SystemContent) { - if ($line -like '*SeServiceLogonRight*') { - $line = [string]::Format('{0},*{1}', $line, $ServiceUserSID); - } - - $NewSystemContent += $line; - } - - Write-IcingaFileSecure -File "$SystemPermissions.inf" -Value $NewSystemContent; - - $SystemOutput = Start-IcingaProcess -Executable 'secedit.exe' -Arguments ([string]::Format('/import /cfg "{0}.inf" /db "{0}.sdb"', $SystemPermissions)); - - if ($SystemOutput.ExitCode -ne 0) { - throw ([string]::Format('Unable to import system permission information: {0}', $SystemOutput.Message)); - return $null; - } - - $SystemOutput = Start-IcingaProcess -Executable 'secedit.exe' -Arguments ([string]::Format('/configure /cfg "{0}.inf" /db "{0}.sdb"', $SystemPermissions)); - - if ($SystemOutput.ExitCode -ne 0) { - throw ([string]::Format('Unable to configure system permission information: {0}', $SystemOutput.Message)); - return $null; - } - - Remove-Item $SystemPermissions*; + Update-IcingaWindowsUserPermission -SID $ServiceUserSID; Test-IcingaAgentServicePermission | Out-Null; } diff --git a/lib/core/icingaagent/tests/Test-IcingaAgentServicePermission.psm1 b/lib/core/icingaagent/tests/Test-IcingaAgentServicePermission.psm1 index 2b46aee..1cc74d1 100644 --- a/lib/core/icingaagent/tests/Test-IcingaAgentServicePermission.psm1 +++ b/lib/core/icingaagent/tests/Test-IcingaAgentServicePermission.psm1 @@ -14,6 +14,12 @@ function Test-IcingaAgentServicePermission() return $TRUE; } + # Never update system SIDs + if ($ServiceUserSID.Length -le 16) { + Write-IcingaTestOutput -Severity 'Passed' -Message ([string]::Format('It seems the provided SID "{0}" is a system SID. Skipping permission check', $ServiceUserSID)); + return $TRUE; + } + if ([string]::IsNullOrEmpty($ServiceUser)) { if (-Not $Silent) { Write-IcingaTestOutput -Severity 'Failed' -Message 'There is no user assigned to the Icinga 2 service or the service is not yet installed'; diff --git a/lib/core/windows/Uninstall-IcingaServiceUser.psm1 b/lib/core/windows/Uninstall-IcingaServiceUser.psm1 index ba75d78..2b8f2c3 100644 --- a/lib/core/windows/Uninstall-IcingaServiceUser.psm1 +++ b/lib/core/windows/Uninstall-IcingaServiceUser.psm1 @@ -11,6 +11,10 @@ function Uninstall-IcingaServiceUser() Write-IcingaConsoleNotice 'Uninstalling user "{0}"' -Objects $IcingaUser; + # Fetch the current service user and SID + $ServiceUser = Get-IcingaServiceUser; + $ServiceUserSID = Get-IcingaUserSID $ServiceUser; + Stop-IcingaService 'icinga2'; Stop-IcingaForWindows; @@ -20,12 +24,9 @@ function Uninstall-IcingaServiceUser() Set-IcingaServiceUser -User 'NT Authority\NetworkService' -Service 'icingapowershell' | Out-Null; Set-IcingaUserPermissions -IcingaUser $IcingaUser -Remove; + Update-IcingaWindowsUserPermission -SID $ServiceUserSID -Remove; - $UserConfig = Remove-IcingaWindowsUser -IcingaUser $IcingaUser; - - if ($null -ne $UserConfig -And ([string]::IsNullOrEmpty($UserConfig.SID) -eq $FALSE)) { - Update-IcingaWindowsUserPermission -SID $UserConfig.SID -Remove; - } + Remove-IcingaWindowsUser -IcingaUser $IcingaUser | Out-Null; Restart-IcingaService 'icinga2'; Restart-IcingaForWindows; diff --git a/lib/core/windows/Update-IcingaWindowsUserPermission.psm1 b/lib/core/windows/Update-IcingaWindowsUserPermission.psm1 index 040c035..654efd1 100644 --- a/lib/core/windows/Update-IcingaWindowsUserPermission.psm1 +++ b/lib/core/windows/Update-IcingaWindowsUserPermission.psm1 @@ -30,28 +30,109 @@ function Update-IcingaWindowsUserPermission() } $SecurityProfile = ''; + # These SID's are required to be present, as they ship with Windows by default + # On non-english Windows installations these SID's might be missing and requires us + # to ensure they are present + [string[]]$RequiredLoginSIDs = @( + '*S-1-5-80-0', # NT SERVICE\ALL SERVICES + '*S-1-5-99-0' # RESTRICTED SERVICES\ALL RESTRICTED SERVICES + ); - if ($Remove -eq $FALSE) { - $SecurityProfile = Get-Content "$UpdatedProfile.inf"; - - foreach ($line in $SecurityProfile) { - if ($line -like '*SeServiceLogonRight*') { - $line = [string]::Format('{0},*{1}', $line, $SID); - } - if ($line -like '*SeDenyNetworkLogonRight*') { - $line = [string]::Format('{0},*{1}', $line, $SID); - } - if ($line -like '*SeDenyInteractiveLogonRight*') { - $line = [string]::Format('{0},*{1}', $line, $SID); - } + # Read the current security profile file + $SecurityProfile = Get-Content "$UpdatedProfile.inf"; + [bool]$IsPrivilegedSection = $FALSE; + [bool]$HasDenyNetworkLogon = $FALSE; + [bool]$HasDenyInteractiveLogon = $FALSE; + foreach ($line in $SecurityProfile) { + if ($line -match '^\s*\[Privilege Rights\]\s*$') { + $IsPrivilegedSection = $TRUE; $NewSecurityProfile += $line; + continue; } - } else { - $SecurityProfile = Get-Content "$UpdatedProfile.inf" -Raw; - $SecurityProfile = $SecurityProfile.Replace([string]::Format(',*{0}', $SID), ''); - $SecurityProfile = $SecurityProfile.Replace([string]::Format('*{0},', $SID), ''); - $NewSecurityProfile = $SecurityProfile; + + # Check for next section, which is [Version] + if ($IsPrivilegedSection -and $line -match '^\s*\[.*\]\s*$') { + $IsPrivilegedSection = $FALSE; + + # If we are adding permissions, ensure the deny logon rights are present + if ($HasDenyNetworkLogon -eq $FALSE) { + Write-IcingaConsoleWarning 'Adding missing "SeDenyNetworkLogonRight" privilege to security profile'; + $NewSecurityProfile += [string]::Format('SeDenyNetworkLogonRight = *{0}', $SID); + } + if ($HasDenyInteractiveLogon -eq $FALSE) { + Write-IcingaConsoleWarning 'Adding missing "SeDenyInteractiveLogonRight" privilege to security profile'; + $NewSecurityProfile += [string]::Format('SeDenyInteractiveLogonRight = *{0}', $SID); + } + } + + if ($line -match '^\s*(SeServiceLogonRight|SeDenyNetworkLogonRight|SeDenyInteractiveLogonRight)\s*=\s*(.*)$') { + [string]$privilegeName = $matches[1]; + [string]$rhsValue = $matches[2]; + + if ($privilegeName -eq 'SeDenyNetworkLogonRight') { + $HasDenyNetworkLogon = $TRUE; + } + if ($privilegeName -eq 'SeDenyInteractiveLogonRight') { + $HasDenyInteractiveLogon = $TRUE; + } + + [string[]]$entryList = @(); + [string[]]$nonSidEntries = @(); + + if ([string]::IsNullOrEmpty($rhsValue) -eq $FALSE) { + [string[]]$tokenArray = $rhsValue -split ','; + + foreach ($token in $tokenArray) { + $token = $token.Trim(); + + if ([string]::IsNullOrEmpty($token) -eq $FALSE) { + # Detect any entries that are not SIDs (SIDs start with '*' and S-1-...) + if (-not ($token -match '^\*S-1-\d+(-\d+)*$')) { + $nonSidEntries += $token; + continue; + } + + if ($Remove -and $token -like "*$SID") { + Write-IcingaConsoleNotice 'Removing SID "{0}" from privilege "{1}"' -Objects $SID, $privilegeName; + continue; + } + + $entryList += $token; + } + } + } + + if ($nonSidEntries.Count -gt 0) { + Write-IcingaConsoleWarning 'Found non-SID entries for "{0}": {1}' -Objects $privilegeName, ($nonSidEntries -join ','); + } + + # Ensure required login SIDs are present for SeServiceLogonRight + if ($privilegeName -eq 'SeServiceLogonRight') { + foreach ($requiredSID in $RequiredLoginSIDs) { + if ($entryList -notcontains $requiredSID) { + Write-IcingaConsoleWarning 'Adding missing default SID "{0}" for privilege "{1}"' -Objects $requiredSID, $privilegeName; + $entryList += $requiredSID; + } + } + } + + # Ensure the managed user SID is present if we are not removing it + if ($Remove -eq $FALSE) { + [string]$managedSidEntry = "*$SID"; + if ($entryList -notcontains $managedSidEntry) { + $entryList += $managedSidEntry; + } + } else { + Write-IcingaConsoleNotice 'Removing SID "{0}" from privilege "{1}"' -Objects $SID, $privilegeName; + } + + # If we add an pricilege and there are no entries left, we still have to add the line with an empty value + # Windows will handle the empty value correctly and remove the line itself + $line = [string]::Format('{0} = {1}', $privilegeName, ($entryList -join ',')); + } + + $NewSecurityProfile += $line; } Set-Content -Path "$UpdatedProfile.inf" -Value $NewSecurityProfile;