Merge pull request #565 from Icinga:fix/more_secure_handling_to_wrote_cache_files

Feature: Improve cache file writer with more robust handling

Ensures that new cache files will always be written inside a `.tmp` file at the same location and tested for validation before applied to the regular file via copy.

This should prevent files becoming corrupt in the future during write operations.

doc/100-General/10-Changelog.md

@@ -29,6 +29,7 @@ Released closed milestones can be found on [GitHub](https://github.com/Icinga/ic
 * [#556](https://github.com/Icinga/icinga-powershell-framework/pull/556) Fixes the certificate folder not present during first installation, preventing permissions properly set from the start which might cause issues once required
 * [#562](https://github.com/Icinga/icinga-powershell-framework/pull/562) Fixes corrupt Performance Data, in case plugins were executed inside a JEA context without the REST-Api
 * [#563](https://github.com/Icinga/icinga-powershell-framework/pull/563) Fixes checks like MSSQL using arguments of type `SecureString` not being usable with the Icinga for Windows REST-Api
+* [#565](https://github.com/Icinga/icinga-powershell-framework/pull/565) Fixes internal cache file writer and reader to store changes inside a `.tmp` file first and validating the file state and content, before applying it to the actual file to prevent data corruption
 
 ### Enhancements
 

lib/core/cache/Copy-IcingaCacheTempFile.psm1

@@ -0,0 +1,12 @@
+function Copy-IcingaCacheTempFile()
+{
+    param (
+        [string]$CacheFile    = '',
+        [string]$CacheTmpFile = ''
+    );
+
+    # Copy the new file over the old one
+    Copy-ItemSecure -Path $CacheTmpFile -Destination $CacheFile -Force | Out-Null;
+    # Remove the old file
+    Remove-ItemSecure -Path $CacheTmpFile -Retries 5 -Force | Out-Null;
+}

lib/core/cache/Get-IcingaCacheData.psm1

@@ -16,6 +16,9 @@
 .PARAMETER KeyName
     This is the actual cache file located under icinga-powershell-framework/cache/<space>/<CacheStore>/<KeyName>.json
     Please note to only provide the name without the '.json' apendix. This is done by the module itself
+.PARAMETER TempFile
+    To safely write data, by default Icinga for Windows will write all content into a .tmp file at the same location with the same name
+    before applying it to the proper file. Set this argument to read the content of a temp file instead
 .INPUTS
     System.String
 .OUTPUTS
@@ -29,13 +32,19 @@ function Get-IcingaCacheData()
     param(
         [string]$Space,
         [string]$CacheStore,
-        [string]$KeyName
+        [string]$KeyName,
+        [switch]$TempFile   = $FALSE
     );
 
     $CacheFile       = Join-Path -Path (Join-Path -Path (Join-Path -Path (Get-IcingaCacheDir) -ChildPath $Space) -ChildPath $CacheStore) -ChildPath ([string]::Format('{0}.json', $KeyName));
     [string]$Content = '';
     $cacheData       = @{ };
 
+    # Read a tmp file if present
+    if ($TempFile) {
+        $CacheFile = [string]::Format('{0}.tmp', $CacheFile);
+    }
+
     if ((Test-Path $CacheFile) -eq $FALSE) {
         return $null;
     }
@@ -46,7 +55,12 @@ function Get-IcingaCacheData()
         return $null;
     }
 
-    $cacheData = ConvertFrom-Json -InputObject ([string]$Content);
+    try {
+        $cacheData = ConvertFrom-Json -InputObject ([string]$Content);
+    } catch {
+        Write-IcingaEventMessage -EventId 1104 -Namespace 'Framework' -ExceptionObject $_ -Objects $CacheFile;
+        return $null;
+    }
 
     if ([string]::IsNullOrEmpty($KeyName)) {
         return $cacheData;

lib/core/cache/Set-IcingaCacheData.psm1

@@ -34,14 +34,19 @@ function Set-IcingaCacheData()
         $Value
     );
 
-    $CacheFile = Join-Path -Path (Join-Path -Path (Join-Path -Path (Get-IcingaCacheDir) -ChildPath $Space) -ChildPath $CacheStore) -ChildPath ([string]::Format('{0}.json', $KeyName));
-    $cacheData = @{ };
+    $CacheFile    = Join-Path -Path (Join-Path -Path (Join-Path -Path (Get-IcingaCacheDir) -ChildPath $Space) -ChildPath $CacheStore) -ChildPath ([string]::Format('{0}.json', $KeyName));
+    $CacheTmpFile = [string]::Format('{0}.tmp', $CacheFile);
+    $cacheData    = @{ };
+
+    if ((Test-IcingaCacheDataTempFile -Space $Space -CacheStore $CacheStore -KeyName $KeyName)) {
+        Copy-IcingaCacheTempFile -CacheFile $CacheFile -CacheTmpFile $CacheTmpFile;
+    }
 
     if ((Test-Path $CacheFile)) {
         $cacheData = Get-IcingaCacheData -Space $Space -CacheStore $CacheStore;
     } else {
         try {
-            New-Item -ItemType File -Path $CacheFile -Force -ErrorAction Stop | Out-Null;
+            New-Item -ItemType File -Path $CacheTmpFile -Force -ErrorAction Stop | Out-Null;
         } catch {
             Exit-IcingaThrowException -InputString $_.Exception -CustomMessage (Get-IcingaCacheDir) -StringPattern 'NewItemUnauthorizedAccessError' -ExceptionType 'Permission' -ExceptionThrown $IcingaExceptions.Permission.CacheFolder;
             Exit-IcingaThrowException -CustomMessage $_.Exception -ExceptionType 'Unhandled' -Force;
@@ -60,5 +65,14 @@ function Set-IcingaCacheData()
         }
     }
 
-    Write-IcingaFileSecure -File $CacheFile -Value (ConvertTo-Json -InputObject $cacheData -Depth 100);
+    # First write all content to a tmp file at the same location, just with '.tmp' at the end
+    Write-IcingaFileSecure -File $CacheTmpFile -Value (ConvertTo-Json -InputObject $cacheData -Depth 100);
+
+    # If something went wrong, remove the cache file again
+    if ((Test-IcingaCacheDataTempFile -Space $Space -CacheStore $CacheStore -KeyName $KeyName) -eq $FALSE) {
+        Remove-ItemSecure -Path $CacheTmpFile -Retries 5 -Force | Out-Null;
+        return;
+    }
+
+    Copy-IcingaCacheTempFile -CacheFile $CacheFile -CacheTmpFile $CacheTmpFile;
 }

lib/core/cache/Test-IcingaCacheDataTempFile.psm1

@@ -0,0 +1,18 @@
+function Test-IcingaCacheDataTempFile()
+{
+    param (
+        [string]$Space,
+        [string]$CacheStore,
+        [string]$KeyName
+    );
+
+    # Once the file is written successully, validate it is fine
+    $tmpContent = Get-IcingaCacheData -Space $Space -CacheStore $CacheStore -KeyName $KeyName -TempFile;
+
+    if ($null -eq $tmpContent) {
+        # File is corrupt or empty
+        return $FALSE;
+    }
+
+    return $TRUE;
+}

lib/core/logging/Icinga_EventLog_Enums.psm1

@@ -38,6 +38,12 @@ if ($null -eq $IcingaEventLogEnums -Or $IcingaEventLogEnums.ContainsKey('Framewo
                 'Details'   = 'Icinga for Windows was unable to run a specific command within the namespace content, to load additional extensions and component data into Icinga for Windows.';
                 'EventId'   = 1103;
             };
+            1104 = @{
+                'EntryType' = 'Error';
+                'Message'   = 'Unable to read Icinga for Windows cache file';
+                'Details'   = 'Icinga for Windows could not read the specified cache file, as the content seems to be corrupt. This happens mostly in case of unexpected shutdowns or terminations during the write process.';
+                'EventId'   = 1104;
+            };
             1400 = @{
                 'EntryType' = 'Error';
                 'Message'   = 'Icinga for Windows background daemon not found';