SSLStream reads invalid data + KB3147458 SSLStream bug (?)

Question

I'm having an issue with SSLStream returning some data when the remote client did not send anything. I am having this issue when the server is listening for a new command. If the server doesn't receive a new request, the ReadMessage() function should catch an IOException due to the Read timeout of the SSLStream. The problem happens when the sslStream.Read() is executed the second time it seems to read 5 bytes which were not sent by the client. So the problem happens in this sequence:

-> ReadMessage() -> sslstream.Read() -> timeout exception caught as expected

-> ReadMessage() -> sslstream.Read() -> timeout exception NOT caught, 5 bytes read even even though the client did not send anything

-> ReadMessage() -> sslstream.Read() -> timeout exception caught as expected

-> ReadMessage() -> sslstream.Read() -> timeout exception NOT caught, 5 bytes read even though client did not send anything...

and so on..

    public void ClientHandle(object obj)
    {
        nRetry = MAX_RETRIES;

        // Open connection with the client
        if (Open() == OPEN_SUCCESS)
        {
            String request = ReadMessage();
            String response = null;

            // while loop for the incoming commands from client
            while (!String.IsNullOrEmpty(request))
            {
                Console.WriteLine("[{0}] {1}", RemoteIPAddress, request);

                response = Execute(request);

                // If QUIT was received, close the connection with the client
                if (response.Equals(QUIT_RESPONSE))
                {
                    // Closing connection
                    Console.WriteLine("[{0}] {1}", RemoteIPAddress, response);

                    // Send QUIT_RESPONSE then return and close this thread
                    SendMessage(response);
                    break;
                }

                // If another command was received, send the response to the client
                if (!response.StartsWith("TIMEOUT"))
                {
                    // Reset nRetry
                    nRetry = MAX_RETRIES;

                    if (!SendMessage(response))
                    {
                        // Couldn't send message
                        Close();
                        break;
                    }
                }


                // Wait for new input request from client
                request = ReadMessage();

                // If nothing was received, SslStream timeout occurred
                if (String.IsNullOrEmpty(request))
                {
                    request = "TIMEOUT";
                    nRetry--;

                    if (nRetry == 0)
                    {
                        // Close everything
                        Console.WriteLine("Client is unreachable. Closing client connection.");
                        Close();
                        break;
                    }
                    else
                    {
                        continue;
                    }
                }
            }

            Console.WriteLine("Stopped");
        }
    }



    public String ReadMessage()
    {
        if (tcpClient != null)
        {
            int bytes = -1;
            byte[] buffer = new byte[MESSAGE_SIZE];

            try
            {
                bytes = sslStream.Read(buffer, 0, MESSAGE_SIZE);
            }
            catch (ObjectDisposedException)
            {
                // Streams were disposed
                return String.Empty;
            }
            catch (IOException)
            {
                return String.Empty;
            }
            catch (Exception)
            {
                // Some other exception occured
                return String.Empty;
            }

            if (bytes != MESSAGE_SIZE)
            {
                return String.Empty;
            }

            // Return string read from the stream
            return Encoding.Unicode.GetString(buffer, 0, MESSAGE_SIZE).Replace("\0", String.Empty);
        }

        return String.Empty;
    }


    public bool SendMessage(String message)
    {
        if (tcpClient != null)
        {
            byte[] data = CreateMessage(message);

            try
            {
                // Write command message to the stream and send it
                sslStream.Write(data, 0, MESSAGE_SIZE);
                sslStream.Flush();
            }
            catch (ObjectDisposedException)
            {
                // Streamers were disposed
                return false;
            }
            catch (IOException)
            {
                // Error while trying to access streams or connection timedout
                return false;
            }
            catch (Exception)
            {
                return false;
            }

            // Data sent successfully
            return true;
        }

        return false;
    }

   private byte[] CreateMessage(String message)
    {
        byte[] data = new byte[MESSAGE_SIZE];

        byte[] messageBytes = Encoding.Unicode.GetBytes(message);

        // Can't exceed MESSAGE_SIZE parameter (max message size in bytes)
        if (messageBytes.Length >= MESSAGE_SIZE)
        {
            throw new ArgumentOutOfRangeException("message", String.Format("Message string can't be longer than {0} bytes", MESSAGE_SIZE));
        }

        for (int i = 0; i < messageBytes.Length; i++)
        {
            data[i] = messageBytes[i];
        }
        for (int i = messageBytes.Length; i < MESSAGE_SIZE; i++)
        {
            data[i] = messageBytes[messageBytes.Length - 1];
        }

        return data;
    }

The very same ReadMessage(), SendMessage() and CreateMessage() functions are used also by the client to send messages to the server. MESSAGE_SIZE constant is also the same and it's set to 2048.

Answer 1

With regard to the SslStream returning five bytes on a Read() after a timeout, this is because the SslStream class doesn't gracefully handle any IOException from the underlying stream, and this is documented as previously noted.

SslStream assumes that a timeout along with any other IOException when one is thrown from the inner stream will be treated as fatal by its caller. Reusing a SslStream instance after a timeout will return garbage. An application should Close the SslStream and throw an exception in these cases.

https://msdn.microsoft.com/en-us/library/system.net.security.sslstream(v=vs.110).aspx

However, you can fix the problem by creating a wrapper class that sits between the Tcp NetworkStream and the SslStream which catches and suppresses the harmless timeout exception, with (seemingly) no loss of functionality.

The full code of this is in my answer on a similar thread, here https://stackoverflow.com/a/48231248/8915494

With regard to the Read() method returning only part of the payload on each Read(), your answer already fixes this correctly. While this is "recent" behaviour for SslStream, it is unfortunately expected behaviour for all networking and all code needs to create some form of buffer to store the fragments until you have a complete packet. For example, if your data exceeds 1500 bytes (the maximum packet size for most Ethernet adapters, assuming Ethernet transport), you are very likely to receive the data in multiple parts and have to reassemble it yourself.

Hope this helps

Answer 2

The problem was that I re-used the SSLStream after a timeout. So I solved the problem just by removing the nRetry variable and set a longer timeout. The related MSDN article says that SSLStream will return garbage after a timeout exception (https://msdn.microsoft.com/en-us/library/system.net.security.sslstream(v=vs.110).aspx):

SslStream assumes that a timeout along with any other IOException when one is thrown from the inner stream will be treated as fatal by its caller. Reusing a SslStream instance after a timeout will return garbage. An application should Close the SslStream and throw an exception in these cases.

Another issue is that Windows update KB3147458 (Windows 10 April's update) changes the something in the behaviour of the Read function. It looks like something in the SSLStream implementation changed and now it returns data in 2 parts, 1 byte and the rest of the bytes every single time. Actually the MSDN document doesn't say that the Read() function will return all the requested bytes in one step and the provided example uses a do-while loop in order to read the exact number of bytes. So I suppose that the Read() function doesn't guarantee to read the exact requested number of bytes all at once, more read iterations might be required.

SSLstream works properly so it's NOT BROKEN. You just need to pay attention and use of a do-while loop and check that all the bytes are read correctly.

I changed the code as shown here to address the bugs I had.

    public String ReadMessage()
    {
        if (tcpClient != null)
        {
            int bytes = -1, offset = 0;
            byte[] buffer = new byte[MESSAGE_SIZE];

            try
            {
                // perform multiple read iterations 
                // and check the number of bytes received
                while (offset < MESSAGE_SIZE)
                {
                    bytes = sslStream.Read(buffer, offset, MESSAGE_SIZE - offset);
                    offset += bytes;

                    if (bytes == 0)
                    {
                        return String.Empty;
                    }
                }
            }
            catch (Exception)
            {
                // Some exception occured
                return String.Empty;
            }

            if (offset != MESSAGE_SIZE)
            {
                return String.Empty;
            }

            // Return string read from the stream
            return Encoding.Unicode.GetString(buffer, 0, MESSAGE_SIZE).Replace("\0", String.Empty);
        }

        return String.Empty;
    }