From e29038b9bd00bc90a922b7c8f78a9b8406c93d39 Mon Sep 17 00:00:00 2001 From: Lord Hepipud Date: Thu, 29 Jan 2026 11:01:26 +0100 Subject: [PATCH] Fixes security issue by restricting access to core folders of Icinga for Windows for generic users --- doc/100-General/10-Changelog.md | 4 + jobs/RenewCertificate.ps1 | 5 + .../Invoke-IcingaForWindowsMigration.psm1 | 1 + .../icingaagent/setters/Set-IcingaAcl.psm1 | 206 +++++++++++++++--- .../setters/Set-IcingaUserPermissions.psm1 | 32 ++- .../tests/Test-IcingaForWindows.psm1 | 22 ++ .../windows/Uninstall-IcingaServiceUser.psm1 | 2 +- 7 files changed, 238 insertions(+), 34 deletions(-) diff --git a/doc/100-General/10-Changelog.md b/doc/100-General/10-Changelog.md index ce36c8c..cfae611 100644 --- a/doc/100-General/10-Changelog.md +++ b/doc/100-General/10-Changelog.md @@ -11,6 +11,10 @@ Released closed milestones can be found on [GitHub](https://github.com/Icinga/ic [Issues and PRs](https://github.com/Icinga/icinga-powershell-framework/milestone/38) +### Security + +* [#850](https://github.com/Icinga/icinga-powershell-framework/pull/850) Fixes security issue, allowing to access the private key file of the Icinga Agent/Icinga for Windows certificate file [GHSA](https://github.com/Icinga/icinga-powershell-framework/security/advisories/GHSA-88h5-rrm6-5973) [CVE-2026-24414](https://www.cve.org/CVERecord?id=CVE-2026-24414) + ### Bugfixes * [#783](https://github.com/Icinga/icinga-powershell-framework/issues/783) Fixes possible FIPS exception on some Windows machines, caused by `MD5` hash algorithm used to verify the service binary file integrity after download instead of `SHA256` diff --git a/jobs/RenewCertificate.ps1 b/jobs/RenewCertificate.ps1 index 6aa9ee4..614dd6c 100644 --- a/jobs/RenewCertificate.ps1 +++ b/jobs/RenewCertificate.ps1 @@ -33,6 +33,11 @@ while ($TRUE) { continue; } + # Let make sure the permissions are updated for the entire Icinga installation + # on a regular basis to ensure system integrity, in case users or other processes + # modify the permissions incorrectly + Set-IcingaUserPermissions; + break; } diff --git a/lib/core/framework/Invoke-IcingaForWindowsMigration.psm1 b/lib/core/framework/Invoke-IcingaForWindowsMigration.psm1 index 735c94e..cd0602c 100644 --- a/lib/core/framework/Invoke-IcingaForWindowsMigration.psm1 +++ b/lib/core/framework/Invoke-IcingaForWindowsMigration.psm1 @@ -183,6 +183,7 @@ function Invoke-IcingaForWindowsMigration() $ServiceUserSID = Get-IcingaUserSID $ServiceUser; Update-IcingaWindowsUserPermission -SID $ServiceUserSID; + Set-IcingaUserPermissions -IcingaUser $ServiceUser; Set-IcingaForWindowsMigration -MigrationVersion (New-IcingaVersionObject -Version '1.14.0'); } diff --git a/lib/core/icingaagent/setters/Set-IcingaAcl.psm1 b/lib/core/icingaagent/setters/Set-IcingaAcl.psm1 index 1bb5f24..1462443 100644 --- a/lib/core/icingaagent/setters/Set-IcingaAcl.psm1 +++ b/lib/core/icingaagent/setters/Set-IcingaAcl.psm1 @@ -1,42 +1,196 @@ +<# +.SYNOPSIS + Configure strict ACLs for a directory so only Administrators, Domain Admins (optional) + and the specified Icinga service user(s) have FullControl. + +.DESCRIPTION + This function validates the provided accounts, disables inheritance on the target + directory, clears existing explicit ACL entries, sets the directory owner and + grants FullControl recursively to the local Administrators group, an optional + Domain Admins group and one or more Icinga service user accounts. + +.PARAMETER Directory + The path to the target directory to update ACLs for. Must be a directory path. + +.PARAMETER Owner + The account to set as owner of the directory. Default is 'Administrators'. + +.PARAMETER IcingaUser + One or more accounts (string or string[]) which should receive FullControl on + the directory. Default is the value returned by `Get-IcingaServiceUser`. + +.PARAMETER DomainName + Optional domain name. When provided the function will also grant FullControl to + '\Domain Admins'. + +.OUTPUTS + None. This function writes progress and errors to the Icinga console helpers. + +.NOTES + - Requires administrative privileges to set ownership and modify ACLs on system + directories. + - Intended for use on Windows hosts only. +#> function Set-IcingaAcl() { - param( - [string]$Directory, - [string]$IcingaUser = (Get-IcingaServiceUser), - [switch]$Remove = $FALSE + param ( + [string]$Directory = $null, + [string]$Owner = 'NT AUTHORITY\SYSTEM', + [string[]]$IcingaUser = (Get-IcingaServiceUser), + [string]$DomainName = ($env:USERDOMAIN).ToLower() ); - if (-Not (Test-Path $Directory)) { - Write-IcingaConsoleWarning 'Unable to set ACL for directory "{0}". Directory does not exist' -Objects $Directory; + # First check if the directory exists + if (-not (Test-Path -Path $Directory -PathType Container)) { + Write-IcingaConsoleWarning -Message 'The folder does not exist: {0}' -Objects $Directory; return; } - if ($IcingaUser.ToLower() -eq 'nt authority\system' -Or $IcingaUser.ToLower() -like '*localsystem') { + # Create the owner account object and validate + try { + $ownerAccount = New-Object System.Security.Principal.NTAccount($Owner); + $ownerAccount.Translate([System.Security.Principal.SecurityIdentifier]) | Out-Null; + } catch { + Write-IcingaConsoleError -Message 'The owner account does not exist or is invalid: {0}' -Objects $Owner; return; } - $DirectoryAcl = (Get-Item -Path $Directory).GetAccessControl('Access'); - $DirectoryAccessRule = New-Object System.Security.AccessControl.FileSystemAccessRule( - $IcingaUser, - 'Modify', - 'ContainerInherit,ObjectInherit', - 'None', - 'Allow' - ); - - if ($Remove -eq $FALSE) { - $DirectoryAcl.SetAccessRule($DirectoryAccessRule); - } else { - foreach ($entry in $DirectoryAcl.Access) { - if (([string]($entry.IdentityReference)).ToLower() -like [string]::Format('*\{0}', $IcingaUser.ToLower())) { - $DirectoryAcl.RemoveAccessRuleSpecific($entry); - } + # Create and validate IcingaUser accounts + foreach ($user in $IcingaUser) { + try { + $userAccount = New-Object System.Security.Principal.NTAccount($user); + $userAccount.Translate([System.Security.Principal.SecurityIdentifier]) | Out-Null; + } catch { + Write-IcingaConsoleError -Message 'The user account does not exist or is invalid: {0}' -Objects $user; + return; } } - Set-Acl -Path $Directory -AclObject $DirectoryAcl; + # Validate if the local Administrators group exists (shouldn't happen anyway) + try { + $adminGroup = New-Object System.Security.Principal.NTAccount('Administrators'); + $adminGroup.Translate([System.Security.Principal.SecurityIdentifier]) | Out-Null; + } catch { + Write-IcingaConsoleError -Message 'The local Administrators group does not exist or is invalid' -Objects $null; + return; + } - if ($Remove -eq $FALSE) { - Test-IcingaAcl -Directory $Directory -WriteOutput | Out-Null; + [bool]$AddDomainAdmins = $TRUE; + + # Validate if the Domain Admins group exists (if DomainName is provided) + if (-not [string]::IsNullOrEmpty($DomainName) ) { + try { + $domainAdminGroup = New-Object System.Security.Principal.NTAccount(([string]::Format('{0}\Domain Admins', $DomainName))); + $domainAdminGroup.Translate([System.Security.Principal.SecurityIdentifier]) | Out-Null; + } catch { + # Continue in this case, just warn the user + Write-IcingaConsoleWarning -Message 'The Domain Admins group does not exist or is invalid: {0}' -Objects ([string]::Format('{0}\Domain Admins', $DomainName)); + $AddDomainAdmins = $FALSE; + } + } + + try { + # Get the ACL for the directory + $acl = Get-Acl -Path $Directory; + + # Now disable inheritance for the parent folder + $acl.SetAccessRuleProtection($true, $false) | Out-Null; + + # Update the owner of the folder to "Administrators" first, to ensure we don't + # run into any exceptions + $acl.SetOwner((New-Object System.Security.Principal.NTAccount('Administrators'))) | Out-Null; + + Write-IcingaConsoleNotice -Message 'Disabled inheritance for directory {0}' -Objects $Directory; + + # Clear all existing ACL entries to ensure we start fresh + $acl.Access | ForEach-Object { + $acl.RemoveAccessRule($_) | Out-Null; + }; + + Write-IcingaConsoleNotice -Message 'Cleared existing ACL entries for directory {0}' -Objects $Directory; + + # Add the permission for each defined user with Full Control + # Only add Icinga user permissions if we are not removing them + foreach ($user in $IcingaUser) { + $rule = New-Object System.Security.AccessControl.FileSystemAccessRule( + $user, + 'FullControl', + 'ContainerInherit, ObjectInherit', + 'None', + 'Allow' + ); + $acl.AddAccessRule($rule) | Out-Null; + } + + # Add local Administrators group (Full Control) + $adminRule = New-Object System.Security.AccessControl.FileSystemAccessRule( + 'Administrators', + 'FullControl', + 'ContainerInherit, ObjectInherit', + 'None', + 'Allow' + ); + + $acl.AddAccessRule($adminRule) | Out-Null; + + # We need to ensure we add Domain Admins, as most likely those will require to have access as well + # and allow them to configure Icinga for Windows + if ($AddDomainAdmins) { + $domainAdminRule = New-Object System.Security.AccessControl.FileSystemAccessRule( + ([string]::Format('{0}\Domain Admins', $DomainName)), + 'FullControl', + 'ContainerInherit, ObjectInherit', + 'None', + 'Allow' + ); + $acl.AddAccessRule($domainAdminRule) | Out-Null; + } + + Write-IcingaConsoleNotice -Message 'Configured new ACL entries for directory {0}' -Objects $Directory; + + # Update the ACL on the directory + Set-Acl -Path $Directory -AclObject $acl | Out-Null; + + # Let's now enable inheritance for all subfolders and files, ensuring they inherit the correct permissions + # from the parent folder and we only require to configure permissions there + Get-ChildItem -Path $Directory -Recurse -Force | ForEach-Object { + try { + $childAcl = Get-Acl -Path $_.FullName; + $childAcl.SetAccessRuleProtection($false, $true) | Out-Null; + # As our parent or current Acl might be owned by SYSTEM, + # we need to set the owner to Administrators here as well to fix exceptions + # for SYSTEM user not being allowed to own this file + $childAcl.SetOwner((New-Object System.Security.Principal.NTAccount('Administrators'))) | Out-Null; + + Set-Acl -Path $_.FullName -AclObject $childAcl | Out-Null; + } catch { + Write-IcingaConsoleWarning -Message 'Failed to enable inheritance for directory {0}: {1}' -Objects $_.FullName, $_.Exception.Message; + } + } + + # Ensure we set the owner of the directory and all sub-items correctly + # Set-Acl cannot do this for the SYSTEM user and ProgramData folders + # properly, so we need to use icacls for this task + $Result = & icacls $Directory /setowner $Owner /T /C | Out-Null; + + if ($LASTEXITCODE -ne 0) { + Write-IcingaConsoleError -Message 'Failed to set owner "{0}" for directory {1} and its sub-items using icacls. Output: {2}' -Objects $Owner, $Directory, $Result; + } + + $StringBuilder = New-Object System.Text.StringBuilder; + $StringBuilder.Append('Permissions for directory "').Append($Directory).Append('" successfully configured for owner "').Append($Owner).Append('"') | Out-Null; + $StringBuilder.Append(' and full access users (').Append(($IcingaUser -join ', ')).Append(')') | Out-Null; + + $StringBuilder.Append(' and groups (Administrators') | Out-Null; + + if ($AddDomainAdmins) { + $StringBuilder.Append(', ').Append(([string]::Format('{0}\Domain Admins)', $DomainName))) | Out-Null; + } else { + $StringBuilder.Append(')') | Out-Null; + }; + + Write-IcingaConsoleNotice -Message $StringBuilder.ToString(); + } catch { + Write-IcingaConsoleError -Message 'Failed to Update ACL for directory {0}: {1}' -Objects $Directory, $_.Exception.Message; } } diff --git a/lib/core/icingaagent/setters/Set-IcingaUserPermissions.psm1 b/lib/core/icingaagent/setters/Set-IcingaUserPermissions.psm1 index 32f4526..499a1aa 100644 --- a/lib/core/icingaagent/setters/Set-IcingaUserPermissions.psm1 +++ b/lib/core/icingaagent/setters/Set-IcingaUserPermissions.psm1 @@ -1,13 +1,31 @@ +<# +.SYNOPSIS + Apply `Set-IcingaAcl` to the common Icinga for Windows directories. + +.DESCRIPTION + Convenience wrapper that calls `Set-IcingaAcl` for a pre-defined set of Icinga + directories (configuration, var, cache and PowerShell config directories). + Use this to consistently apply the same ACL rules to the standard locations. + +.PARAMETER IcingaUser + The account or accounts to grant FullControl. Defaults to the value returned + by `Get-IcingaServiceUser`. + +.OUTPUTS + None. Operates by calling `Set-IcingaAcl` which emits its own console output. + +.NOTES + - Requires administrative privileges to change ACLs on system folders. +#> function Set-IcingaUserPermissions() { param ( - [string]$IcingaUser = (Get-IcingaServiceUser), - [switch]$Remove = $FALSE + [string]$IcingaUser = (Get-IcingaServiceUser) ); - Set-IcingaAcl -Directory "$Env:ProgramData\icinga2\etc" -IcingaUser $IcingaUser -Remove:$Remove; - Set-IcingaAcl -Directory "$Env:ProgramData\icinga2\var" -IcingaUser $IcingaUser -Remove:$Remove; - Set-IcingaAcl -Directory (Get-IcingaCacheDir) -IcingaUser $IcingaUser -Remove:$Remove; - Set-IcingaAcl -Directory (Get-IcingaPowerShellConfigDir) -IcingaUser $IcingaUser -Remove:$Remove; - Set-IcingaAcl -Directory (Join-Path -Path (Get-IcingaFrameworkRootPath) -ChildPath 'certificate') -IcingaUser $IcingaUser -Remove:$Remove; + Set-IcingaAcl -Directory "$Env:ProgramData\icinga2\etc" -IcingaUser $IcingaUser; + Set-IcingaAcl -Directory "$Env:ProgramData\icinga2\var" -IcingaUser $IcingaUser; + Set-IcingaAcl -Directory (Get-IcingaCacheDir) -IcingaUser $IcingaUser; + Set-IcingaAcl -Directory (Get-IcingaPowerShellConfigDir) -IcingaUser $IcingaUser; + Set-IcingaAcl -Directory (Join-Path -Path (Get-IcingaFrameworkRootPath) -ChildPath 'certificate') -IcingaUser $IcingaUser; } diff --git a/lib/core/icingaagent/tests/Test-IcingaForWindows.psm1 b/lib/core/icingaagent/tests/Test-IcingaForWindows.psm1 index e6621ff..8b1403c 100644 --- a/lib/core/icingaagent/tests/Test-IcingaForWindows.psm1 +++ b/lib/core/icingaagent/tests/Test-IcingaForWindows.psm1 @@ -1,3 +1,25 @@ +<# +.SYNOPSIS + Run a set of environment checks for Icinga for Windows and optionally fix issues. + +.DESCRIPTION + Performs multiple validation checks for the local Icinga for Windows installation, + including presence of services, service users, file permissions, certificates, + REST API availability and other environment items. When the `-ResolveProblems` + switch is used the function will attempt to repair certain detected issues. + +.PARAMETER ResolveProblems + When provided, the function will attempt to fix detected problems where the + code includes a repair implementation (for example repairing services, updating + ACLs, renewing certificates). Use with caution as fixes may change system state. + +.OUTPUTS + None. Results are conveyed using the framework's `Write-IcingaTestOutput` helpers. + +.NOTES + - Some repair actions require administrative privileges. + - Designed to run on Windows systems where Icinga for Windows is installed. +#> function Test-IcingaForWindows() { param ( diff --git a/lib/core/windows/Uninstall-IcingaServiceUser.psm1 b/lib/core/windows/Uninstall-IcingaServiceUser.psm1 index 2b8f2c3..e0651f2 100644 --- a/lib/core/windows/Uninstall-IcingaServiceUser.psm1 +++ b/lib/core/windows/Uninstall-IcingaServiceUser.psm1 @@ -23,7 +23,7 @@ function Uninstall-IcingaServiceUser() Set-IcingaServiceUser -User 'NT Authority\NetworkService' -Service 'icinga2' | Out-Null; Set-IcingaServiceUser -User 'NT Authority\NetworkService' -Service 'icingapowershell' | Out-Null; - Set-IcingaUserPermissions -IcingaUser $IcingaUser -Remove; + Set-IcingaUserPermissions -IcingaUser 'NT Authority\NetworkService'; Update-IcingaWindowsUserPermission -SID $ServiceUserSID -Remove; Remove-IcingaWindowsUser -IcingaUser $IcingaUser | Out-Null;