Skip to content

asyncio: fix SSL connections by using native TLS transport#884

Open
roydahan wants to merge 1 commit into
scylladb:masterfrom
roydahan:fix-asyncio-ssl
Open

asyncio: fix SSL connections by using native TLS transport#884
roydahan wants to merge 1 commit into
scylladb:masterfrom
roydahan:fix-asyncio-ssl

Conversation

@roydahan
Copy link
Copy Markdown
Collaborator

Summary

  • Replace SimpleStrategy with NetworkTopologyStrategy across the codebase (ScyllaDB dropped SimpleStrategy support)
  • Fix AsyncioConnection SSL support by using asyncio's native TLS transport (loop.create_connection(ssl=)) instead of ssl.SSLContext.wrap_socket(), which Python 3.8+ rejects in asyncio's sock_sendall/sock_recv
  • This resolves the "protocol version 21 not supported" errors when connecting to TLS-requiring ScyllaDB clusters (the TLS Alert byte 0x15 was misread as protocol version 21)

Details

SimpleStrategy → NetworkTopologyStrategy

All CQL CREATE KEYSPACE statements, Python strategy objects, and test assertions updated. The SimpleStrategy class definition in cassandra/metadata.py is preserved for backward compatibility.

AsyncioConnection SSL Fix

Problem: Python 3.8+ explicitly rejects ssl.SSLSocket in asyncio.loop.sock_sendall() and sock_recv() with TypeError("ssl.SSLSocket is not supported"). The base Connection._connect_socket() wraps the socket with ssl.SSLContext.wrap_socket(), producing an SSLSocket that asyncio refuses.

Solution: Override _connect_socket() in AsyncioConnection to skip SSL wrapping, keeping a plain TCP socket. After the socket connects (preserving shard-aware port binding), upgrade to TLS asynchronously via loop.create_connection(sock=, ssl=). A new _AsyncioProtocol class bridges asyncio's transport/protocol API back to Connection.process_io_buffer() for reading. Writes use transport.write() for SSL connections. Non-SSL connections are unchanged.

Edge cases handled: SSL handshake failure properly unblocks the write coroutine and defuncts the connection.

Fixes #330

@roydahan roydahan force-pushed the fix-asyncio-ssl branch 2 times, most recently from c4b4b48 to 99e7245 Compare May 11, 2026 18:38
@dkropachev
Copy link
Copy Markdown
Collaborator

@roydahan , please rebase

@roydahan
Copy link
Copy Markdown
Collaborator Author

Rebased.
Last run of CI passed successfully includeing the asyncio failed tests.

@roydahan
Copy link
Copy Markdown
Collaborator Author

Can anyone review this PR?

Copy link
Copy Markdown
Collaborator

@dkropachev dkropachev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like the right direction for asyncio TLS: using asyncio's native transport/protocol layer avoids the ssl.SSLSocket incompatibility with sock_recv() / sock_sendall().

I think it would be worth tightening the design before merge. Right now TLS and plain CQL use different I/O models, and TLS writes do not appear to account for asyncio transport backpressure. My main suggestion would be to consider using one transport/protocol path for both TLS and non-TLS asyncio connections in this PR.

The reason I think it belongs here is that the hard part is not only "make TLS work"; it is introducing asyncio transports into this reactor correctly. Once the reactor has transport lifecycle, protocol callbacks, write flow control, close handling, and tests for those pieces, keeping plain CQL on the old socket-coroutine path leaves two separate implementations of the same connection state machine. That makes future behavior harder to reason about and increases the chance that fixes land in one path but not the other.

CI already gives broad connectivity coverage through the asyncio integration matrix, including an SSL scenario via TestSslThroughNlb, so I would focus new tests on the new transport mechanics and compatibility edge cases rather than adding another generic SSL integration test.

Comment thread cassandra/io/asyncioreactor.py
Comment thread cassandra/io/asyncioreactor.py
Comment thread cassandra/io/asyncioreactor.py
Comment thread cassandra/io/asyncioreactor.py Outdated
Comment thread cassandra/io/asyncioreactor.py
Comment thread cassandra/io/asyncioreactor.py Outdated
Comment thread cassandra/io/asyncioreactor.py Outdated
Comment thread cassandra/io/asyncioreactor.py
Python 3.8+ rejects ssl.SSLSocket in asyncio's sock_sendall/sock_recv
with TypeError. This caused the driver to fail connecting to ScyllaDB
clusters requiring TLS, manifesting as 'protocol version 21 not
supported' errors (0x15 = TLS Alert byte misread as protocol version).

Fix by using asyncio's native TLS transport (loop.create_connection
with ssl= parameter) instead of wrapping sockets with
ssl.SSLContext.wrap_socket(). This preserves shard-aware port binding
done during _initiate_connection().

Add _AsyncioProtocol to bridge asyncio's transport/protocol API back
to Connection.process_io_buffer() for SSL data reads. Non-SSL
connections continue using the existing sock_recv path.

Fixes scylladb#330
@roydahan
Copy link
Copy Markdown
Collaborator Author

All comments were addressed.

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

Successfully merging this pull request may close these issues.

Asyncio backend: SSL doesn't work

2 participants