Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with Websocket+TLS in Poco 1.13.3 #4524

Closed
acarioni opened this issue Apr 8, 2024 · 7 comments
Closed

Issue with Websocket+TLS in Poco 1.13.3 #4524

acarioni opened this issue Apr 8, 2024 · 7 comments
Assignees
Labels

Comments

@acarioni
Copy link

acarioni commented Apr 8, 2024

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?

@acarioni acarioni added the bug label Apr 8, 2024
@matejk matejk self-assigned this Apr 11, 2024
@matejk
Copy link
Contributor

matejk commented Apr 12, 2024

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
Copy link
Author

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
Copy link
Contributor

matejk commented Apr 12, 2024

@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
Copy link
Author

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
Copy link
Contributor

matejk commented Apr 12, 2024

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
Copy link
Author

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
Copy link
Member

aleks-f commented Apr 12, 2024

@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.

@aleks-f aleks-f added question and removed bug labels Apr 13, 2024
@pocoproject pocoproject locked and limited conversation to collaborators Apr 13, 2024
@aleks-f aleks-f converted this issue into discussion #4527 Apr 13, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Projects
None yet
Development

No branches or pull requests

3 participants