Issue with Websocket+TLS in Poco 1.13.3

[acarioni]

I'm encountering an issue with a client program that establishes a connection to a Websocket server, designed to send and receive messages at any time.

After updating to Poco 1.13.3, the program hangs when TLS is enabled. It functions correctly without TLS and worked without issues in Poco 1.13.2.

A stripped-down version of the program looks like this:

class Writer : public Poco::Runnable
{
  WebSocket& _ws;
public:
  Writer(WebSocket ws) : _ws(ws) {}

  virtual void run()
  {
    int i = 1;
    while (1)
    {
      Poco::Thread::sleep(1000);
      std::stringstream ss;
      ss << "hello " << i++;
      string s = ss.str();
      _ws.sendFrame(s.data(), s.size());
    }
  }
};

class Reader : public Poco::Runnable
{
  WebSocket& _ws;
public:
  Reader(WebSocket ws) : _ws(ws) {}

  virtual void run()
  {
    int flags, n;
    Poco::Buffer<char> buf(1024);
    while (1)
    {
      buf.resize(0);
      n = _ws.receiveFrame(buf, flags);
      if (n == 0 && flags == 0)
      {
        break;
      }
      if (n > 0 && (flags & 0xf) == Poco::Net::WebSocket::FrameOpcodes::FRAME_OP_TEXT)
      {
        string output(buf.begin(), buf.end());
        cout << output << "\n";
      }
    }
  }
};

int main() {
  // HTTPClientSession cs("echo.websocket.in", 80); // this is OK
  
  Poco::Net::Context::Ptr pContext = new Poco::Net::Context(Poco::Net::Context::TLS_CLIENT_USE, "", "", "", Poco::Net::Context::VERIFY_RELAXED, 9, true, "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
  HTTPSClientSession cs(pContext);
  cs.setHost("echo.websocket.in");
  cs.setPort(443);
  
  HTTPRequest request(HTTPRequest::HTTP_GET, "/", HTTPRequest::HTTP_1_1);
  HTTPResponse response;
  WebSocket ws(cs, request, response);

  Reader rdr(ws);
  Poco::Thread tr;
  tr.start(rdr);

  Writer wrt(ws);
  Poco::Thread tw;
  tw.start(wrt);

  tr.join();
  tw.join();
}

Expected Output:

hello 1
hello 2
...

I suspect the problem may be linked to issue #4435, which introduced a mutex to prevent simultaneous read and write operations on a secure socket. This way however when a reader blocks holding the mutex, no one can write.

Could this be a bug related to the changes in Poco 1.13.3, or is there a possibility that I'm misusing the Websocket API?

[matejk]

In the example shown above there is only one WebSocket instantiated and then used in reader and writer inside the same process. Reader wants to read data using ::SSL_read in function receiveBytes and writer wants to send the data using the same instance of websocket (sendBytes).

Mutex to protect SSL members is locked by reader and therefore the writer can't send.

I doubt that this is a valid use case at all. Reader and writer shall have their own instances of web socket and not share the same one. @aleks-f ?

[acarioni]

To clarify, my project involves a WebSocket client that is designed to be receptive to messages from the server at any moment. Given this requirement, it's practical to have a dedicated thread constantly monitoring the WebSocket connection for incoming data. Further, the same client must be capable of sending messages at any time, as it interacts with user-generated input. This design rationale underpins the structure of my code example, which I hope is now more understandable.

The scenario I've described is typical for applications that produce and consume real-time data and the straightforward implementation I've provided is effective when using a plain WebSocket. Problems arise when I try to use a Secure WebSocket because reading from the WebSocket prevents any writing.

[matejk]

@acarioni , would it be possible for you to have two instances of websockets: one for reading and another completely independent for writing to the server?

[acarioni]

Do you mean two Websocket connections, one for reading and one for writing, or two Websocket objects sharing the same connection?

In my case, the first alternative is not acceptable but the second one is.

[matejk]

Having two Websocket objects would definitely solve locking issue. Is it possible for you to test whether it work properly on one connection in your situation?

[acarioni]

After implementing the suggested changes, the program still hangs.

The reason is that both ws and ws2 (see the code below) reference the same SecureSocketImpl instance. Consequently, this shared instance's mutex becomes a contention point, as it is simultaneously accessed by SecureSocketImpl::receiveBytes and SecureSocketImpl::sendBytes.

  // same as before ...
  WebSocket ws(cs, request, response);
  WebSocket ws2(ws);

  Reader rdr(ws);
  Poco::Thread tr;
  tr.start(rdr);

  Writer wrt(ws2);
  Poco::Thread tw;
  tw.start(wrt);

  tr.join();
  tw.join();

[aleks-f]

@acarioni what you are doing is not thread-safe, you can't safely access the same SSL object from multiple threads:

openssl/openssl#20446 (comment)

You should decouple those threads with notification queues and do the I/O in one thread only.

[acarioni]

If a single thread must handle all operations involving the same WebSocket, then setting a timeout on each potentially blocking operation becomes necessary. And if a timeout occurs, this event can be opportunistically used to initiate the subsequent operation.

In my case, this approach might look like this:

virtual void run() {
  WebSocket ws;
  ws.setReceiveTimeout(???);

  while (1) {
    try {
      ws.receiveFrame();
    } catch (TimeoutException) {
      // timeout: check if there are pending writing operations
      pNF = queue.dequeueNotification();
      if (pNf) {
        ws.sendFrame();
      }
      continue; // read next frame if available
    }
    // no timeout: process the frame ...
  }
}

Is that what you're suggesting?

[lanthora]

BUG repair destroying the original function is a very bad experience.

The read operation could have used Poco::Net::Socket::select to determine whether there is readable data before locking. This can make the lock range as small as possible. This can fix the problem without breaking the API.

[Mike4Online]

I just confirmed that my application is broken with Poco 1.13.3 due to the new mutex -- see SecureSocketStream is not thread safe #4435. When I downgrade to Poco 1.13.2 my application runs fine.

My application guards secure and insecure WebSocket sends with its own mutexes, which are no longer required with Poco 1.13.3 now that SecureSocketStream is internally thread-safe. But the new mutex in Poco 1.13.3 imposes stricter adherence to the rule of sending only via the send thread. This new restriction broke my app, which otherwise ran fine on Poco 1.13.2 and older (sending and receiving from the receive thread in certain cases; e.g. directly sending PONG responses when a PING is received, and protecting those calls with its own mutex).

I assume other applications which use Poco have implemented their own mutex schemes up until Poco 1.13.3, and that those mutex schemes permitted thread-safe sending and receiving from the same thread. As such, it would have been helpful to list this change under a "Breaking Changes" heading.

The GitHub Release page for 1.13.3 lists this new mutex under Bug Fixes and Improvements:

GH #4435 SecureStreamSocket is not thread-safe

No indication this is a potentially breaking change ^^^.

Full disclosure: my application was not directly broken by Poco 1.13.3. Rather it was the WAMP_POCO library upon which my application depends. Specifically, the method ApplicationWebSocket::receiveMessage -- which is intended to be the receive thread -- calls Poco's sendFrame method to respond to CLOSE and PING messages. Unfortunately, the WAMP_POCO project hasn't had a commit in 8 years; I'll have to update the code myself to be compatible with Poco 1.13.3.

Incidentally, I have to build Poco on 5 platforms (Windows, macOS, Linux, 32-bit and 64-bit), so I use vcpkg to do so. But I'll have to build Poco 1.13.2 manually on each of these platforms since vcpkg never issued a port for Poco version 1.13.2.

[sappho192]

I just confirmed that my application is broken with Poco 1.13.3 due to the new mutex -- see SecureSocketStream is not thread safe #4435. When I downgrade to Poco 1.13.2 my application runs fine.

I stumbled upon this issue and solved my program's weird behavior by downgrading to 1.13.2.