Fixes IfW API to properly read client streams complete, even when not fully send

LordHepipud · LordHepipud · commit 749c30e37f4f · 2024-04-09T17:58:37.000+02:00
diff --git a/doc/100-General/10-Changelog.md b/doc/100-General/10-Changelog.md
@@ -17,6 +17,7 @@ Released closed milestones can be found on [GitHub](https://github.com/Icinga/ic
 
 ### Bugfixes
 
+* [#672](https://github.com/Icinga/icinga-powershell-framework/pull/issues) Fixes Icinga for Windows REST-Api to fully read client data, even when they client is sending the packets on a very slow basis, preventing the API trying to process an incomplete request
 * [#707](https://github.com/Icinga/icinga-powershell-framework/pull/707) Fixes size of the `Icinga for Windows` eventlog by setting it to `20MiB`, allowing to store more events before they are overwritten
 * [#710](https://github.com/Icinga/icinga-powershell-framework/pull/710) Fixes various console errors while running Icinga for Windows outside of an administrative shell
 * [#713](https://github.com/Icinga/icinga-powershell-framework/pull/713) Fixes Icinga for Windows REST-Api which fails during certificate auth handling while running as `NT Authority\NetworkService`
diff --git a/lib/daemon/Set-IcingaForWindowsThreadAlive.psm1 b/lib/daemon/Set-IcingaForWindowsThreadAlive.psm1
@@ -6,7 +6,8 @@ function Set-IcingaForWindowsThreadAlive()
         $ThreadPool                 = $null,
         [hashtable]$ThreadArgs      = @{ },
         [switch]$Active             = $FALSE,
-        [hashtable]$TerminateAction = @{ }
+        [hashtable]$TerminateAction = @{ },
+        [int]$Timeout               = 300
     );
 
     if ([string]::IsNullOrEmpty($ThreadName)) {
@@ -27,6 +28,7 @@ function Set-IcingaForWindowsThreadAlive()
                 'ThreadPool'      = $ThreadPool;
                 'Active'          = [bool]$Active;
                 'TerminateAction' = $TerminateAction;
+                'Timeout'         = $Timeout;
             }
         );
 
@@ -36,4 +38,5 @@ function Set-IcingaForWindowsThreadAlive()
     $Global:Icinga.Public.ThreadAliveHousekeeping[$ThreadName].LastSeen        = [DateTime]::Now;
     $Global:Icinga.Public.ThreadAliveHousekeeping[$ThreadName].Active          = [bool]$Active;
     $Global:Icinga.Public.ThreadAliveHousekeeping[$ThreadName].TerminateAction = $TerminateAction;
+    $Global:Icinga.Public.ThreadAliveHousekeeping[$ThreadName].Timeout         = $Timeout;
 }
diff --git a/lib/daemon/Suspend-IcingaForWindowsFrozenThreads.psm1 b/lib/daemon/Suspend-IcingaForWindowsFrozenThreads.psm1
@@ -12,7 +12,7 @@ function Suspend-IcingaForWindowsFrozenThreads()
             }
 
             # Check if the thread is active and not doing something for 5 minutes
-            if (([DateTime]::Now - $ThreadConfig.LastSeen).TotalSeconds -lt 300) {
+            if (([DateTime]::Now - $ThreadConfig.LastSeen).TotalSeconds -lt $ThreadConfig.Timeout) {
                 continue;
             }
 
diff --git a/lib/daemons/RestAPI/daemon/New-IcingaForWindowsRESTApi.psm1 b/lib/daemons/RestAPI/daemon/New-IcingaForWindowsRESTApi.psm1
@@ -125,6 +125,8 @@ function New-IcingaForWindowsRESTApi()
         try {
             $NextRESTApiThreadId = (Get-IcingaNextRESTApiThreadId);
 
+            Write-IcingaDebugMessage -Message 'Scheduling Icinga for Windows API request' -Objects 'REST-Thread Id', $NextRESTApiThreadId;
+
             if ($Global:Icinga.Public.Daemons.RESTApi.ApiRequests.ContainsKey($NextRESTApiThreadId) -eq $FALSE) {
                 Close-IcingaTCPConnection -Connection $Connection;
                 $Connection = $null;
diff --git a/lib/daemons/RestAPI/threads/Get-IcingaNextRESTApiThreadId.psm1 b/lib/daemons/RestAPI/threads/Get-IcingaNextRESTApiThreadId.psm1
@@ -3,6 +3,8 @@ function Get-IcingaNextRESTApiThreadId()
     # Improve our thread management by distributing new REST requests to a non-active thread
     [array]$ConfiguredThreads = $Global:Icinga.Public.ThreadAliveHousekeeping.Keys;
 
+    Write-IcingaDebugMessage -Message 'Distributing Icinga for Windows REST-Api calls to one of those threads' -Objects 'REST-Thread Ids', ($ConfiguredThreads | Out-String);
+
     foreach ($thread in $ConfiguredThreads) {
         if ($thread.ToLower() -NotLike 'Start-IcingaForWindowsRESTThread::New-IcingaForWindowsRESTThread::CheckThread::*') {
             continue;
diff --git a/lib/daemons/RestAPI/threads/New-IcingaForWindowsRESTThread.psm1 b/lib/daemons/RestAPI/threads/New-IcingaForWindowsRESTThread.psm1
@@ -16,14 +16,23 @@ function New-IcingaForWindowsRESTThread()
                 continue;
             }
 
+            Write-IcingaDebugMessage -Message 'Icinga for Windows REST-Api thread is processing a request' -Objects 'REST-Thread Id', $ThreadId;
+
             # block sleeping until content available
             $Connection = $Global:Icinga.Public.Daemons.RESTApi.ApiRequests.$ThreadId.Take();
 
+            # Set our thread being active before we start reading the TCP stream, as we otherwise might find ourself
+            # in a process were we block our API entirely, because all reqests are scheduled to one single thread
+            Set-IcingaForWindowsThreadAlive -ThreadName $Global:Icinga.Protected.ThreadName -Active -Timeout 6 -TerminateAction @{ 'Command' = 'Close-IcingaTCPConnection'; 'Arguments' = @{ 'Connection' = $Connection } };
+
             # Read the received message from the stream by using our smart functions
             [string]$RestMessage = Read-IcingaTCPStream -Client $Connection.Client -Stream $Connection.Stream;
             # Now properly translate the entire rest message to a parsable hashtable
             $RESTRequest         = Read-IcingaRESTMessage -RestMessage $RestMessage -Connection $Connection;
 
+            # Once we read all of our messages, reset the thread alive with the default timeout
+            Set-IcingaForWindowsThreadAlive -ThreadName $Global:Icinga.Protected.ThreadName -Active -TerminateAction @{ 'Command' = 'Close-IcingaTCPConnection'; 'Arguments' = @{ 'Connection' = $Connection } };
+
             if ($null -ne $RESTRequest) {
 
                 # Check if we require to authenticate the user
@@ -62,9 +71,6 @@ function New-IcingaForWindowsRESTThread()
                     }
                 }
 
-                # Set our thread being active
-                Set-IcingaForWindowsThreadAlive -ThreadName $Global:Icinga.Protected.ThreadName -Active -TerminateAction @{ 'Command' = 'Close-IcingaTCPConnection'; 'Arguments' = @{ 'Connection' = $Connection } };
-
                 # We should remove clients from the blacklist who are sending valid requests
                 Remove-IcingaRESTClientBlacklist -Client $Connection.Client -ClientList $Global:Icinga.Public.Daemons.RESTApi.ClientBlacklist;
                 switch (Get-IcingaRESTPathElement -Request $RESTRequest -Index 0) {
diff --git a/lib/daemons/ServiceCheckDaemon/daemon/Add-IcingaServiceCheckDaemon.psm1 b/lib/daemons/ServiceCheckDaemon/daemon/Add-IcingaServiceCheckDaemon.psm1
@@ -17,7 +17,7 @@ function Add-IcingaServiceCheckDaemon()
         $RegisteredServices = Get-IcingaRegisteredServiceChecks;
 
         # Debugging message
-        Write-IcingaDebugMessage 'Found these service checks to load within service check daemon: {0}' -Objects ($RegisteredServices.Keys | Out-String);
+        Write-IcingaDebugMessage 'Found these service checks to load within service check daemon' -Objects ($RegisteredServices.Keys | Out-String);
 
         # Loop all found background services and create a new thread for each check
         foreach ($service in $RegisteredServices.Keys) {
diff --git a/lib/webserver/Icinga_HTTPResponse_Enums.psm1 b/lib/webserver/Icinga_HTTPResponse_Enums.psm1
@@ -10,6 +10,7 @@
     403 = 'Forbidden';
     404 = 'Not Found'
     500 = 'Internal Server Error';
+    504 = 'Gateway Timeout';
 };
 
 [hashtable]$HTTPResponseType = @{
@@ -19,6 +20,7 @@
     'Forbidden'             = 403;
     'Not Found'             = 404;
     'Internal Server Error' = 500;
+    'Gateway Timeout'       = 504;
 };
 
 <#
diff --git a/lib/webserver/Read-IcingaRESTMessage.psm1 b/lib/webserver/Read-IcingaRESTMessage.psm1
@@ -71,16 +71,69 @@ function Read-IcingaRESTMessage()
     # Body
     $RestMessage -match '(\{(.*\n)*}|\{.*\})' | Out-Null;
 
-    if ($null -ne $Matches) {
+    # Store your messag body inside a stringbuilder object
+    $MessageBody = New-Object 'System.Text.StringBuilder';
+
+    # Our message is not complete
+    if ($null -eq $Matches) {
+        # Try to figure out if we received parts of your body from the already read message
+        $RestMsgArray    = $RestMessage.Split("`r`n");
+        [bool]$EmptyLine = $FALSE;
+        [bool]$BeginBody = $FALSE;
+
+        foreach ($entry in $RestMsgArray) {
+            if ($BeginBody) {
+                $MessageBody.Append($entry) | Out-Null;
+                continue;
+            }
+
+            if ([string]::IsNullOrEmpty($entry)) {
+                Write-IcingaDebugMessage `
+                    -Message 'Found end of header, lets check for body elements';
+
+                # In case we found two empty lines in a row, we found our body
+                if ($EmptyLine) {
+                    $BeginBody = $TRUE;
+                    continue;
+                }
+
+                # A first empty line means we found a possible header end
+                $EmptyLine = $TRUE;
+            } else {
+                #Reset the empty line in case the next line contains content
+                $EmptyLine = $FALSE;
+            }
+        }
+
+        Write-IcingaDebugMessage `
+            -Message 'We partially received the body of the message, but it might be incomplete. Lets check. Read body length' `
+            -Objects $MessageBody.Length, $MessageBody.ToString();
+    } else {
+        # Our message is already complete
         $Request.Add('Body', $Matches[1]);
     }
 
     # We received a content length, but couldn't load the body. Some clients will send the body as separate message
     # Lets try to read the body content
     if ($null -ne $Connection) {
         if ($Request.ContainsKey('ContentLength') -And $Request.ContentLength -gt 0 -And ($Request.ContainsKey('Body') -eq $FALSE -Or [string]::IsNullOrEmpty($Request.Body))) {
-            $Request.Body = Read-IcingaTCPStream -Client $Connection.Client -Stream $Connection.Stream -ReadLength $Request.ContentLength;
-            Write-IcingaDebugMessage -Message 'Body Content' -Objects $Request;
+
+            Write-IcingaDebugMessage `
+                -Message 'The message body was not send or incomplete. Received body size and content' `
+                -Objects $MessageBody.Length, $MessageBody.ToString();
+
+            # In case we received party of the message earlier, read the remaining bytes from the message
+            # and append it to our body
+            $MessageBody.Append(
+                (Read-IcingaTCPStream -Client $Connection.Client -Stream $Connection.Stream -ReadLength ($Request.ContentLength - $MessageBody.Length))
+            ) | Out-Null;
+
+            # Add our fully read message to our body object
+            $Request.Body = $MessageBody.ToString();
+
+            # Some debug output
+            Write-IcingaDebugMessage -Message 'Full request dump' -Objects $Request;
+            Write-IcingaDebugMessage -Message 'Body Content' -Objects $Request.Body;
         }
     }
 
diff --git a/lib/webserver/Read-IcingaTCPStream.psm1 b/lib/webserver/Read-IcingaTCPStream.psm1
@@ -6,19 +6,118 @@ function Read-IcingaTCPStream()
         [int]$ReadLength                       = 0
     );
 
+    [bool]$UnknownMessageSize = $FALSE;
+    [int]$TimeoutInSeconds    = 5;
+
     if ($ReadLength -eq 0) {
-        $ReadLength = $Client.ReceiveBufferSize;
+        $ReadLength         = $Client.ReceiveBufferSize;
+        $UnknownMessageSize = $TRUE;
     }
 
     if ($null -eq $Stream) {
         return $null;
     }
 
+    # Ensure that our stream will timeout after a while to not block
+    # the API permanently
+    $Stream.ReadTimeout    = $TimeoutInSeconds * 1000;
     # Get the maxium size of our buffer
-    [byte[]]$bytes   = New-Object byte[] $ReadLength;
-    # Read the content of our SSL stream
-    $MessageSize     = $Stream.Read($bytes, 0, $ReadLength);
-    # Resize our array to the correct size
+    [int]$MessageSize      = 0;
+    [byte[]]$bytes         = New-Object byte[] $ReadLength;
+    [int]$DebugReadLength  = $ReadLength;
+    [int]$EmptyStreamCount = 0;
+    # Ensure we calculate our own timeouts for the API, in case some malicious actions take place
+    # and we only receive one byte each second for a large message. We should terminate the request
+    # in this case
+    $ReadTimeout           = [DateTime]::Now;
+
+    while ($ReadLength -ne 0) {
+        # Create a buffer element to store our message size into
+        [byte[]]$buffer = New-Object byte[] $ReadLength;
+        # Read the content of our SSL stream
+        $ReadBytes      = $Stream.Read($buffer, 0, $ReadLength);
+        # Now lets copy all read bytes from our buffer to our bytes variable
+        # As we might read multiple times from our stream, we have to read
+        # the entire buffer from index 0 and copy our content to our bytes
+        # variable, while our starting index is always at the last message
+        # sizes end (Message size equals ReadBytes from the previous attempt)
+        # and we need to copy as much bytes to our new array, as we read
+        [array]::Copy($buffer, 0, $bytes, $MessageSize, $ReadBytes);
+
+        $ReadLength  -= $ReadBytes;
+        $MessageSize += $ReadBytes;
+
+        # In case the client terminates the session, we might receive a bunch of invalid
+        # information. Just to make sure there is no other error happening,
+        # we should wait a little to check if the client is still present and
+        # trying to send data
+        if ($ReadBytes -eq 0) {
+            $EmptyStreamCount += 1;
+            Start-Sleep -Milliseconds 100;
+        }
+
+        if ($EmptyStreamCount -gt 20) {
+            # This would mean we waited 2 seconds to receive actual data, the client decide not to do anything
+            # We should terminate this session
+
+            Send-IcingaTCPClientMessage -Message (
+                New-IcingaTCPClientRESTMessage `
+                    -HTTPResponse ($IcingaHTTPEnums.HTTPResponseType.'Internal Server Error') `
+                    -ContentBody @{ 'message' = 'The Icinga for Windows API received no data while reading the TCP stream. The session was terminated to protect the API.' }
+            ) -Stream $Stream;
+
+            Close-IcingaTCPConnection -Connection @{ 'Client' = $Client; 'Stream' = $Stream; };
+
+            Write-IcingaDebugMessage `
+                -Message 'Icinga for Windows API received multiple empty results and attempts from the client. Terminating session.' `
+                -Objects $DebugReadLength, $ReadBytes, $ReadLength, $MessageSize;
+
+            # Return an empty JSON
+            return '{ }';
+        }
+
+        if ((([DateTime]::Now) - $ReadTimeout).TotalSeconds -gt $TimeoutInSeconds -And $ReadLength -ne 0) {
+            # This would mean we waited to long to receive the entire message
+            # We should terminate this session, in case we have't just completed the read
+            # of our message
+
+            Send-IcingaTCPClientMessage -Message (
+                New-IcingaTCPClientRESTMessage `
+                    -HTTPResponse ($IcingaHTTPEnums.HTTPResponseType.'Gateway Timeout') `
+                    -ContentBody @{ 'message' = 'The Icinga for Windows API waited too long to receive the entire message from the client. The session was terminated to protect the API.' }
+            ) -Stream $Stream;
+
+            Close-IcingaTCPConnection -Connection @{ 'Client' = $Client; 'Stream' = $Stream; };
+
+            Write-IcingaDebugMessage `
+                -Message 'Icinga for Windows API waited too long to receive the entire message from the client. Terminating session.' `
+                -Objects $DebugReadLength, $ReadBytes, $ReadLength, $MessageSize;
+
+            # Return an empty JSON
+            return '{ }';
+        }
+
+        # In case our client did not set a defined message size, we just take the default from the Client
+        # In most cases, the client will default to 65536 as network buffer for this and we should just
+        # pretend the message was send properly. In most cases, the important request will go through
+        # as this will only happen to the first message. Afterwards we can use the HTTP headers to read
+        # the actual content size of the message
+        if ($UnknownMessageSize) {
+            Write-IcingaDebugMessage `
+                -Message 'Message buffer for incoming network stream was 65536. Assuming we read all data' `
+                -Objects $DebugReadLength, $ReadBytes, $ReadLength, $MessageSize;
+
+            break;
+        }
+
+        # Just some debug context, allowing us to check what is going on with the API
+        Write-IcingaDebugMessage `
+            -Message 'Network stream output while parsing request. Request Size / Read Bytes / Remaining Size / Message Size' `
+            -Objects $DebugReadLength, $ReadBytes, $ReadLength, $MessageSize;
+    }
+
+    # Resize our array to the correct size, as we might have read
+    # 65536 bytes because of our client not sending the correct length
     [byte[]]$resized = New-Object byte[] $MessageSize;
     [array]::Copy($bytes, 0, $resized, 0, $MessageSize);
 

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ function Suspend-IcingaForWindowsFrozenThreads()`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`# Check if the thread is active and not doing something for 5 minutes`
`15`		`- if (([DateTime]::Now - $ThreadConfig.LastSeen).TotalSeconds -lt 300) {`
	`15`	`+ if (([DateTime]::Now - $ThreadConfig.LastSeen).TotalSeconds -lt $ThreadConfig.Timeout) {`
`16`	`16`	`continue;`
`17`	`17`	`}`
`18`	`18`