Skip to content

Commit b14fe37

Browse files
committed
connection: fail closed on pyopenssl verification
1 parent 8d6fdc5 commit b14fe37

8 files changed

Lines changed: 77 additions & 16 deletions

File tree

CHANGELOG.rst

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
11
Unreleased
22
==========
33

4+
Bug Fixes
5+
---------
6+
* Explicit ``ssl_options={}`` and endpoint SSL options now enable TLS instead
7+
of being treated as omitted SSL options. An empty dict encrypts the
8+
connection without server certificate verification unless verification
9+
options such as ``ca_certs`` or ``cert_reqs`` are supplied.
10+
* Legacy ``ssl_options`` now work with the Twisted and Eventlet reactors by
11+
using pyOpenSSL contexts with mapped protocol, verification, cipher, SNI, and
12+
hostname-validation settings.
13+
* ``SSLContext`` setup now applies ``cert_reqs`` through ``verify_mode`` instead
14+
of overwriting the SSL options bitmask, preserving default hardening flags.
15+
416
Others
517
------
618
* Message serialization now receives the connection's negotiated ``ProtocolFeatures``:

cassandra/cluster.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,15 @@ def default_retry_policy(self, policy):
885885
886886
The following documentation only applies when ssl_options is used without ssl_context.
887887
888+
.. versionchanged:: 3.29.12
889+
890+
An explicit empty dict (``ssl_options={}``) enables TLS with default
891+
options, while ``ssl_options=None`` leaves the connection in plaintext.
892+
Empty options do not supply ``ca_certs`` or ``cert_reqs``, so the
893+
encrypted connection does not verify the server certificate. Supply
894+
``ca_certs`` and, when needed, ``'check_hostname': True`` to authenticate
895+
the server.
896+
888897
By default, a ``ca_certs`` value should be supplied (the value should be
889898
a string pointing to the location of the CA certs file), and you probably
890899
want to specify ``ssl_version`` as ``ssl.PROTOCOL_TLS`` to match

cassandra/connection.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,6 @@ def _pyopenssl_ssl_method_from_stdlib(ssl_module, ssl_version):
168168
def _pyopenssl_verify_mode_from_cert_reqs(ssl_module, cert_reqs):
169169
if cert_reqs is None:
170170
return None
171-
if cert_reqs.__class__ is not type(ssl.CERT_REQUIRED):
172-
return cert_reqs
173171
if cert_reqs == ssl.CERT_NONE:
174172
return ssl_module.VERIFY_NONE
175173
if cert_reqs in (ssl.CERT_OPTIONAL, ssl.CERT_REQUIRED):
@@ -293,6 +291,10 @@ def _pyopenssl_cert_common_names(cert):
293291

294292

295293
def _validate_pyopenssl_hostname(cert, hostname):
294+
if cert is None:
295+
raise ssl.CertificateError(
296+
"peer did not present a certificate; cannot verify hostname %r" % (hostname,))
297+
296298
san_dns_names, san_ip_addresses = _pyopenssl_cert_subject_alt_names(cert)
297299
san_names = san_dns_names + san_ip_addresses
298300
hostname_is_ip = _is_ip_address(hostname)

cassandra/io/eventletreactor.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@
2020
from eventlet.queue import Queue
2121
from greenlet import GreenletExit
2222
import logging
23+
import ssl
2324
from threading import Event
2425
import time
2526

26-
from cassandra.connection import (Connection, ConnectionShutdown, Timer, TimerManager,
27+
from cassandra.connection import (Connection, ConnectionException, ConnectionShutdown, Timer, TimerManager,
2728
_build_pyopenssl_context_from_options as _build_pyopenssl_context,
2829
_default_pyopenssl_ssl_method,
2930
_ensure_pyopenssl_context_requires_verification,
@@ -132,7 +133,11 @@ def _initiate_connection(self, sockaddr):
132133

133134
def _validate_hostname(self):
134135
expected_name = (self.ssl_options or {}).get('server_hostname') or self.endpoint.address
135-
_validate_pyopenssl_hostname(self._socket.get_peer_certificate(), expected_name)
136+
try:
137+
_validate_pyopenssl_hostname(self._socket.get_peer_certificate(), expected_name)
138+
except ssl.CertificateError as exc:
139+
raise ConnectionException(
140+
"Hostname verification failed: %s" % (exc,), self.endpoint) from exc
136141

137142
def _build_ssl_context_from_options(self):
138143
_check_pyopenssl()

cassandra/io/twistedreactor.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,15 +189,19 @@ def verify_callback(self, connection, x509, errnum, errdepth, ok):
189189

190190
@staticmethod
191191
def info_callback(connection, where, ret):
192-
if where & SSL.SSL_CB_HANDSHAKE_DONE:
193-
tls_protocol = connection.get_app_data()
194-
app_data = getattr(tls_protocol, _TLS_APP_DATA_ATTR, None)
195-
if app_data and app_data.check_hostname:
196-
try:
197-
_validate_pyopenssl_hostname(connection.get_peer_certificate(), app_data.expected_name)
198-
except ssl.CertificateError as exc:
199-
tls_protocol.failVerification(
200-
Failure(ConnectionException(str(exc), app_data.endpoint)))
192+
if not where & SSL.SSL_CB_HANDSHAKE_DONE:
193+
return
194+
tls_protocol = connection.get_app_data()
195+
app_data = getattr(tls_protocol, _TLS_APP_DATA_ATTR, None)
196+
if not (app_data and app_data.check_hostname):
197+
return
198+
try:
199+
_validate_pyopenssl_hostname(
200+
connection.get_peer_certificate(), app_data.expected_name)
201+
except Exception as exc:
202+
tls_protocol.failVerification(
203+
Failure(ConnectionException(
204+
"Hostname verification failed: %s" % (exc,), app_data.endpoint)))
201205

202206
def clientConnectionForTLS(self, tlsProtocol):
203207
connection = SSL.Connection(self.context, None)

tests/unit/io/test_eventletreactor.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
eventletreactor = None
3232
EventletConnection = None # noqa
3333

34-
from cassandra.connection import Connection, DefaultEndPoint
34+
from cassandra.connection import Connection, ConnectionException, DefaultEndPoint
3535

3636
CA_CERTS = os.path.abspath(os.path.join(
3737
os.path.dirname(__file__), '..', '..', 'integration', 'long', 'ssl', 'rootCa.crt'))
@@ -205,7 +205,7 @@ def test_validate_hostname_prefers_san_over_common_name(self):
205205
conn._socket.get_peer_certificate.return_value = _make_certificate(
206206
'node.example.com', san_dns_names=['other.example.com'])
207207

208-
with self.assertRaises(ssl.CertificateError):
208+
with self.assertRaises(ConnectionException):
209209
conn._validate_hostname()
210210

211211

tests/unit/io/test_twistedreactor.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from types import SimpleNamespace
1919
from unittest.mock import Mock, patch
2020

21-
from cassandra.connection import Connection, DefaultEndPoint
21+
from cassandra.connection import Connection, ConnectionException, DefaultEndPoint
2222

2323
try:
2424
from twisted.test import proto_helpers
@@ -235,6 +235,24 @@ def set_tlsext_host_name(self, server_hostname):
235235
validate_hostname.assert_called_once_with(
236236
first_connection.peer_cert, 'node1.example.com')
237237

238+
def test_info_callback_fails_closed_on_unexpected_exception(self):
239+
connection = Mock()
240+
tls_protocol = Mock()
241+
setattr(tls_protocol, twistedreactor._TLS_APP_DATA_ATTR,
242+
twistedreactor._TLSAppData(
243+
DefaultEndPoint('node.example.com'), 'node.example.com', True))
244+
connection.get_app_data.return_value = tls_protocol
245+
connection.get_peer_certificate.return_value = object()
246+
247+
with patch.object(twistedreactor, '_validate_pyopenssl_hostname',
248+
side_effect=RuntimeError('boom')):
249+
twistedreactor._SSLCreator.info_callback(
250+
connection, twistedreactor.SSL.SSL_CB_HANDSHAKE_DONE, 1)
251+
252+
failure = tls_protocol.failVerification.call_args[0][0]
253+
assert isinstance(failure.value, ConnectionException)
254+
assert "Hostname verification failed" in str(failure.value)
255+
238256

239257
class TestTwistedTimer(TimerTestMixin, unittest.TestCase):
240258
"""

tests/unit/test_connection.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@ def test_ip_hostname_accepts_exact_common_name_without_san(self):
126126

127127
_validate_pyopenssl_hostname(cert, '1.2.3.4')
128128

129+
def test_rejects_missing_certificate(self):
130+
with self.assertRaises(ssl.CertificateError):
131+
_validate_pyopenssl_hostname(None, 'node.example.com')
132+
129133

130134
class PyOpenSSLContextTest(unittest.TestCase):
131135

@@ -148,6 +152,13 @@ def test_check_hostname_promotes_verify_none_to_verify_peer(self):
148152

149153
assert context.verify_mode == FakePyOpenSSLModule.VERIFY_PEER
150154

155+
def test_plain_int_cert_reqs_option_is_translated(self):
156+
context = _build_pyopenssl_context_from_options(
157+
FakePyOpenSSLModule,
158+
{'cert_reqs': int(ssl.CERT_REQUIRED)})
159+
160+
assert context.verify_mode == FakePyOpenSSLModule.VERIFY_PEER
161+
151162
def test_supplied_context_check_hostname_promotes_verify_none_to_verify_peer(self):
152163
context = FakePyOpenSSLContext(FakePyOpenSSLModule.TLS_CLIENT_METHOD)
153164
context.verify_mode = FakePyOpenSSLModule.VERIFY_NONE

0 commit comments

Comments
 (0)