Skip to content

Commit 704dde9

Browse files
committed
connection: preserve explicit empty ssl_options
Problem: several SSL paths used ssl_options truthiness, so explicit ssl_options={} was treated like omitted ssl_options=None. That disabled SSL setup in connection/reactor paths, skipped the ssl_options deprecation warning, and made Insights report SSL as disabled. Fix: preserve whether ssl_options was supplied and expose a shared SSL-enabled predicate. Build default SSL contexts for explicit ssl_options on the standard path, use pyOpenSSL contexts for Twisted/Eventlet, and update Cluster and Insights checks to use explicit None comparisons. Add focused unit coverage for connection setup, client routes, Twisted/Eventlet, and Insights reporting.
1 parent e605de2 commit 704dde9

11 files changed

Lines changed: 203 additions & 37 deletions

File tree

cassandra/cluster.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,8 @@ def __init__(self,
12981298

12991299
if cloud is not None:
13001300
self.cloud = cloud
1301-
if contact_points is not _NOT_SET or endpoint_factory or ssl_context or ssl_options:
1301+
if (contact_points is not _NOT_SET or endpoint_factory or
1302+
ssl_context is not None or ssl_options is not None):
13021303
raise ValueError("contact_points, endpoint_factory, ssl_context, and ssl_options "
13031304
"cannot be specified with a cloud configuration")
13041305

@@ -1508,7 +1509,7 @@ def __init__(self,
15081509

15091510
self.metrics_enabled = metrics_enabled
15101511

1511-
if ssl_options and not ssl_context:
1512+
if ssl_options is not None and ssl_context is None:
15121513
warn('Using ssl_options without ssl_context is '
15131514
'deprecated and will result in an error in '
15141515
'the next major release. Please use ssl_context '

cassandra/connection.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,7 @@ class Connection(object):
803803
endpoint = None
804804
ssl_options = None
805805
ssl_context = None
806+
_ssl_options_explicit = False
806807
last_error = None
807808

808809
# The current number of operations that are in flight. More precisely,
@@ -885,7 +886,11 @@ def __init__(self, host='127.0.0.1', port=9042, authenticator=None,
885886
self.endpoint = host if isinstance(host, EndPoint) else DefaultEndPoint(host, port)
886887

887888
self.authenticator = authenticator
888-
self.ssl_options = ssl_options.copy() if ssl_options else {}
889+
endpoint_ssl_options = self.endpoint.ssl_options
890+
# Explicit ssl_options={} enables SSL with default options; omitted
891+
# ssl_options=None leaves SSL disabled unless an endpoint supplies options.
892+
self._ssl_options_explicit = ssl_options is not None
893+
self.ssl_options = ssl_options.copy() if ssl_options is not None else {}
889894
self.ssl_context = ssl_context
890895
self.sockopts = sockopts
891896
self.compression = compression
@@ -905,10 +910,11 @@ def __init__(self, host='127.0.0.1', port=9042, authenticator=None,
905910
self._on_orphaned_stream_released = on_orphaned_stream_released
906911
self._application_info = application_info
907912

908-
if ssl_options:
909-
self.ssl_options.update(self.endpoint.ssl_options or {})
910-
elif self.endpoint.ssl_options:
911-
self.ssl_options = self.endpoint.ssl_options
913+
if ssl_options is not None:
914+
self.ssl_options.update(endpoint_ssl_options or {})
915+
elif endpoint_ssl_options is not None:
916+
self._ssl_options_explicit = True
917+
self.ssl_options = endpoint_ssl_options
912918

913919
# PYTHON-1331
914920
#
@@ -918,7 +924,7 @@ def __init__(self, host='127.0.0.1', port=9042, authenticator=None,
918924
#
919925
# Note the use of pop() here; we are very deliberately removing these params from ssl_options if they're present. After this
920926
# operation ssl_options should contain only args needed for the ssl_context.wrap_socket() call.
921-
if not self.ssl_context and self.ssl_options:
927+
if self.ssl_context is None and self._ssl_options_explicit:
922928
self.ssl_context = self._build_ssl_context_from_options()
923929

924930
self.max_request_id = min(self.max_in_flight - 1, (2 ** 15) - 1)
@@ -942,6 +948,10 @@ def host(self):
942948
def port(self):
943949
return self.endpoint.port
944950

951+
@property
952+
def _ssl_enabled(self):
953+
return self.ssl_context is not None or self._ssl_options_explicit
954+
945955
@classmethod
946956
def initialize_reactor(cls):
947957
"""

cassandra/datastax/insights/reporter.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,13 @@ def _get_startup_data(self):
141141

142142
cert_validation = None
143143
try:
144-
if self._session.cluster.ssl_context:
144+
if self._session.cluster.ssl_context is not None:
145145
if isinstance(self._session.cluster.ssl_context, ssl.SSLContext):
146146
cert_validation = self._session.cluster.ssl_context.verify_mode == ssl.CERT_REQUIRED
147147
else: # pyopenssl
148148
from OpenSSL import SSL
149149
cert_validation = self._session.cluster.ssl_context.get_verify_mode() != SSL.VERIFY_NONE
150-
elif self._session.cluster.ssl_options:
150+
elif self._session.cluster.ssl_options is not None:
151151
cert_validation = self._session.cluster.ssl_options.get('cert_reqs') == ssl.CERT_REQUIRED
152152
except Exception as e:
153153
log.debug('Unable to get the cert validation: {}'.format(e))
@@ -186,7 +186,8 @@ def _get_startup_data(self):
186186
'compression': compression_type.upper() if compression_type else 'NONE',
187187
'reconnectionPolicy': insights_registry.serialize(self._session.cluster.reconnection_policy),
188188
'sslConfigured': {
189-
'enabled': bool(self._session.cluster.ssl_options or self._session.cluster.ssl_context),
189+
'enabled': (self._session.cluster.ssl_context is not None or
190+
self._session.cluster.ssl_options is not None),
190191
'certValidation': cert_validation
191192
},
192193
'authProvider': {

cassandra/io/asyncioreactor.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ def __init__(self, *args, **kwargs):
133133
Connection.__init__(self, *args, **kwargs)
134134
self._background_tasks = set()
135135
self._transport = None
136-
self._using_ssl = bool(self.ssl_context)
136+
self._using_ssl = self._ssl_enabled
137137

138138
self._connect_socket()
139139
self._socket.setblocking(0)

cassandra/io/eventletreactor.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,22 @@ def _check_pyopenssl():
4343
)
4444

4545

46+
def _build_pyopenssl_context_from_options(ssl_options):
47+
context = SSL.Context(ssl_options.get('ssl_version', SSL.TLSv1_METHOD))
48+
if 'certfile' in ssl_options:
49+
context.use_certificate_file(ssl_options['certfile'])
50+
if 'keyfile' in ssl_options:
51+
context.use_privatekey_file(ssl_options['keyfile'])
52+
if 'ca_certs' in ssl_options:
53+
context.load_verify_locations(ssl_options['ca_certs'])
54+
if 'cert_reqs' in ssl_options:
55+
context.set_verify(
56+
ssl_options['cert_reqs'],
57+
callback=lambda _connection, _x509, _errnum, _errdepth, ok: ok
58+
)
59+
return context
60+
61+
4662
class EventletConnection(Connection):
4763
"""
4864
An implementation of :class:`.Connection` that utilizes ``eventlet``.
@@ -92,7 +108,7 @@ def service_timeouts(cls):
92108

93109
def __init__(self, *args, **kwargs):
94110
Connection.__init__(self, *args, **kwargs)
95-
self.uses_legacy_ssl_options = self.ssl_options and not self.ssl_context
111+
self.uses_legacy_ssl_options = False
96112
self._write_queue = Queue()
97113

98114
self._connect_socket()
@@ -114,7 +130,7 @@ def _initiate_connection(self, sockaddr):
114130
super(EventletConnection, self)._initiate_connection(sockaddr)
115131
else:
116132
self._socket.connect(sockaddr)
117-
if self.ssl_context or self.ssl_options:
133+
if self._ssl_enabled:
118134
self._socket.do_handshake()
119135

120136
def _match_hostname(self):
@@ -126,6 +142,10 @@ def _match_hostname(self):
126142
raise Exception("Hostname verification failed! Certificate name '{}' "
127143
"doesn't endpoint '{}'".format(cert_name, self.endpoint.address))
128144

145+
def _build_ssl_context_from_options(self):
146+
_check_pyopenssl()
147+
return _build_pyopenssl_context_from_options(self.ssl_options)
148+
129149
def close(self):
130150
with self.lock:
131151
if self.is_closed:

cassandra/io/twistedreactor.py

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,30 @@
3939
log = logging.getLogger(__name__)
4040

4141

42+
def _check_pyopenssl():
43+
if not _HAS_SSL:
44+
raise ImportError(
45+
str(import_exception) +
46+
', pyOpenSSL must be installed to enable SSL support with the Twisted event loop'
47+
)
48+
49+
50+
def _build_pyopenssl_context_from_options(ssl_options):
51+
context = SSL.Context(ssl_options.get("ssl_version", SSL.TLSv1_METHOD))
52+
if "certfile" in ssl_options:
53+
context.use_certificate_file(ssl_options["certfile"])
54+
if "keyfile" in ssl_options:
55+
context.use_privatekey_file(ssl_options["keyfile"])
56+
if "ca_certs" in ssl_options:
57+
context.load_verify_locations(ssl_options["ca_certs"])
58+
if "cert_reqs" in ssl_options:
59+
context.set_verify(
60+
ssl_options["cert_reqs"],
61+
callback=lambda _connection, _x509, _errnum, _errdepth, ok: ok
62+
)
63+
return context
64+
65+
4266
def _cleanup(cleanup_weakref):
4367
try:
4468
cleanup_weakref()._cleanup()
@@ -141,25 +165,14 @@ def _on_loop_timer(self):
141165
class _SSLCreator(object):
142166
def __init__(self, endpoint, ssl_context, ssl_options, check_hostname, timeout):
143167
self.endpoint = endpoint
144-
self.ssl_options = ssl_options
168+
self.ssl_options = ssl_options or {}
145169
self.check_hostname = check_hostname
146170
self.timeout = timeout
147171

148-
if ssl_context:
172+
if ssl_context is not None:
149173
self.context = ssl_context
150174
else:
151-
self.context = SSL.Context(SSL.TLSv1_METHOD)
152-
if "certfile" in self.ssl_options:
153-
self.context.use_certificate_file(self.ssl_options["certfile"])
154-
if "keyfile" in self.ssl_options:
155-
self.context.use_privatekey_file(self.ssl_options["keyfile"])
156-
if "ca_certs" in self.ssl_options:
157-
self.context.load_verify_locations(self.ssl_options["ca_certs"])
158-
if "cert_reqs" in self.ssl_options:
159-
self.context.set_verify(
160-
self.ssl_options["cert_reqs"],
161-
callback=self.verify_callback
162-
)
175+
self.context = _build_pyopenssl_context_from_options(self.ssl_options)
163176
self.context.set_info_callback(self.info_callback)
164177

165178
def verify_callback(self, connection, x509, errnum, errdepth, ok):
@@ -217,27 +230,27 @@ def __init__(self, *args, **kwargs):
217230
self._loop.maybe_start()
218231

219232
def _check_pyopenssl(self):
220-
if self.ssl_context or self.ssl_options:
221-
if not _HAS_SSL:
222-
raise ImportError(
223-
str(import_exception) +
224-
', pyOpenSSL must be installed to enable SSL support with the Twisted event loop'
225-
)
233+
if self._ssl_enabled:
234+
_check_pyopenssl()
235+
236+
def _build_ssl_context_from_options(self):
237+
_check_pyopenssl()
238+
return _build_pyopenssl_context_from_options(self.ssl_options)
226239

227240
def add_connection(self):
228241
"""
229242
Convenience function to connect and store the resulting
230243
connector.
231244
"""
232245
host, port = self.endpoint.resolve()
233-
if self.ssl_context or self.ssl_options:
246+
if self._ssl_enabled:
234247
# Can't use optionsForClientTLS here because it *forces* hostname verification.
235248
# Cool they enforce strong security, but we have to be able to turn it off
236249
self._check_pyopenssl()
237250

238251
ssl_connection_creator = _SSLCreator(
239252
self.endpoint,
240-
self.ssl_context if self.ssl_context else None,
253+
self.ssl_context if self.ssl_context is not None else None,
241254
self.ssl_options,
242255
self._check_hostname,
243256
self.connect_timeout,

tests/unit/advanced/test_insights.py

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,22 @@
1616
import unittest
1717

1818
import logging
19+
import ssl
1920
import sys
20-
from unittest.mock import sentinel
21+
from unittest.mock import Mock, sentinel
2122

2223
from cassandra import ConsistencyLevel
2324
from cassandra.cluster import (
2425
ExecutionProfile, GraphExecutionProfile, GraphAnalyticsExecutionProfile
2526
)
27+
from cassandra.connection import Connection
2628
from cassandra.datastax.graph.query import GraphOptions
29+
from cassandra.datastax.insights.reporter import MonitorReporter
2730
from cassandra.datastax.insights.registry import insights_registry
2831
from cassandra.datastax.insights.serializers import initialize_registry
2932
from cassandra.policies import (
3033
LoadBalancingPolicy,
34+
HostDistance,
3135
DCAwareRoundRobinPolicy,
3236
TokenAwarePolicy,
3337
WhiteListRoundRobinPolicy,
@@ -88,6 +92,60 @@ def superclass_sentinel_serializer(obj):
8892
# with default -- same behavior
8993
assert insights_registry.serialize(SubclassSentinel(), default=object()) is sentinel.serialized_superclass
9094

95+
96+
class TestMonitorReporterStartupData(unittest.TestCase):
97+
98+
def _get_startup_data(self, ssl_options=None, ssl_context=None):
99+
reporter = MonitorReporter.__new__(MonitorReporter)
100+
reporter._interval = 30
101+
102+
connection = Mock()
103+
connection.host = '127.0.0.1'
104+
connection._compression_type = 'NONE'
105+
connection._socket.getsockname.return_value = ('127.0.0.1', 9042)
106+
107+
cluster = Mock()
108+
cluster.auth_provider = None
109+
cluster.client_id = 'client-id'
110+
cluster.connection_class = Connection
111+
cluster.control_connection._connection = connection
112+
cluster.application_name = None
113+
cluster.application_version = None
114+
cluster._endpoint_map_for_insights = {}
115+
cluster.idle_heartbeat_interval = 30
116+
cluster.metadata.all_hosts.return_value = []
117+
cluster.profile_manager.distance.return_value = HostDistance.LOCAL
118+
cluster.protocol_version = 4
119+
cluster.reconnection_policy = object()
120+
cluster.ssl_context = ssl_context
121+
cluster.ssl_options = ssl_options
122+
123+
session = Mock()
124+
session.cluster = cluster
125+
session.hosts = []
126+
session.session_id = 'session-id'
127+
128+
reporter._session = session
129+
return reporter._get_startup_data()
130+
131+
def test_empty_ssl_options_reported_as_enabled(self):
132+
startup_data = self._get_startup_data(ssl_options={})
133+
134+
assert startup_data['data']['sslConfigured']['enabled'] is True
135+
136+
def test_omitted_ssl_options_reported_as_disabled(self):
137+
startup_data = self._get_startup_data()
138+
139+
assert startup_data['data']['sslConfigured']['enabled'] is False
140+
141+
def test_ssl_context_reported_as_enabled(self):
142+
ssl_context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
143+
144+
startup_data = self._get_startup_data(ssl_context=ssl_context)
145+
146+
assert startup_data['data']['sslConfigured']['enabled'] is True
147+
148+
91149
class TestConfigAsDict(unittest.TestCase):
92150

93151
# graph/query.py

tests/unit/io/test_eventletreactor.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,28 @@
2323

2424
try:
2525
from eventlet import monkey_patch
26+
from cassandra.io import eventletreactor
2627
from cassandra.io.eventletreactor import EventletConnection
2728
except (ImportError, AttributeError):
29+
eventletreactor = None
2830
EventletConnection = None # noqa
2931

32+
from cassandra.connection import Connection, DefaultEndPoint
33+
34+
35+
@unittest.skipIf(EventletConnection is None, "eventlet is not available")
36+
@unittest.skipIf(not getattr(eventletreactor, '_PYOPENSSL', False), "pyOpenSSL is not available")
37+
class EventletSSLContextTest(unittest.TestCase):
38+
39+
def test_empty_ssl_options_build_pyopenssl_context(self):
40+
conn = EventletConnection.__new__(EventletConnection)
41+
42+
Connection.__init__(conn, DefaultEndPoint('1.2.3.4'), ssl_options={})
43+
44+
assert conn._ssl_enabled
45+
assert conn.ssl_context is not None
46+
47+
3048
skip_condition = EventletConnection is None or EVENT_LOOP_MANAGER != "eventlet"
3149
# There are some issues with some versions of pypy and eventlet
3250
@notpypy

tests/unit/io/test_twistedreactor.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,3 +196,19 @@ def test_push(self, mock_connectTCP):
196196
self.obj_ut.push('123 pickup')
197197
self.mock_reactor_cft.assert_called_with(
198198
transport_mock.write, '123 pickup')
199+
200+
@unittest.skipIf(not getattr(twistedreactor, '_HAS_SSL', False), "pyOpenSSL is not available")
201+
@patch('cassandra.io.twistedreactor.connectProtocol')
202+
@patch('cassandra.io.twistedreactor.TCP4ClientEndpoint')
203+
@patch('cassandra.io.twistedreactor.SSL4ClientEndpoint')
204+
def test_empty_ssl_options_use_ssl_endpoint(self, mock_ssl_endpoint, mock_tcp_endpoint, mock_connect_protocol):
205+
conn = twistedreactor.TwistedConnection(
206+
DefaultEndPoint('1.2.3.4'),
207+
cql_version='3.0.1',
208+
ssl_options={})
209+
210+
conn.add_connection()
211+
212+
mock_ssl_endpoint.assert_called_once()
213+
mock_tcp_endpoint.assert_not_called()
214+
mock_connect_protocol.assert_called_once()

0 commit comments

Comments
 (0)