Skip to content
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

WebSocket connections fail when NATS server is configured with TLS verification. #442

Closed
allanbank opened this issue Apr 6, 2023 · 0 comments

Comments

@allanbank
Copy link
Contributor

When using a WesSocket URI to connect to the NATS server and the server is configured to verify client TLS connections via the NATS protocol the server sends the tls_required=true flag as part of the initial INFO message.

That results in the WebSocketTransport's connect_tls(...) method getting called with the hostname instead of the full URL. That gets passed to the to the aiohttp library and eventually results in a exception like:

aiohttp.client_exceptions.InvalidURL: localhost

This can be reproduced with the following patch:

diff --git a/tests/utils.py b/tests/utils.py
index 711e6f5..cd00a65 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -477,7 +477,7 @@ class SingleWebSocketTLSServerTestCase(unittest.TestCase):
         )
 
         server = NATSD(
-            port=4222, config_file=get_config_file("conf/ws_tls.conf")
+            port=4222, tls=True, config_file=get_config_file("conf/ws_tls.conf")
         )
         self.server_pool.append(server)
         for natsd in self.server_pool:

The fundamental issue is that for the WebSocket connections the TLS upgrade is not supported and should not be attempted.

I think there are two potential solutions:

  1. Update the Client. _process_connect_init(self) to not do the tls_required processing on ws or wss connections.
  2. Update the WebSocketTransport to detect the second connect[_tls]() call and ignore it if already connected via TLS or throw an exception if not connected via TLS.

I am partial to the second option and will work on a pull request to implement that but I am happy to be pointed in a different direction.

allanbank added a commit to allanbank/nats.py that referenced this issue Apr 6, 2023
allanbank added a commit to allanbank/nats.py that referenced this issue Apr 6, 2023
…nnection to TLS.

If the connection is already using TLS then continue. Otherwise raise a ProtocolError.

Fixes nats-io#442
allanbank added a commit to allanbank/nats.py that referenced this issue Apr 6, 2023
allanbank added a commit to allanbank/nats.py that referenced this issue Apr 6, 2023
…nnection to TLS.

If the connection is already using TLS then continue. Otherwise raise a ProtocolError.

Fixes nats-io#442
@bruth bruth closed this as completed in 990d30f May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant