Skip to content

Commit 74a9b82

Browse files
committed
Revert "ssl: move pyopenssl hostname validation to follow-up"
This reverts commit f8c3ca4.
1 parent f8c3ca4 commit 74a9b82

5 files changed

Lines changed: 251 additions & 9 deletions

File tree

cassandra/connection.py

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from functools import wraps, partial, total_ordering
1919
from heapq import heappush, heappop
2020
import io
21+
import ipaddress
2122
import logging
2223
import socket
2324
import struct
@@ -56,6 +57,103 @@
5657
log = logging.getLogger(__name__)
5758

5859

60+
def _pyopenssl_cert_to_ssl_cert(peer_cert):
61+
from cryptography import x509
62+
from cryptography.x509.oid import NameOID
63+
64+
cert = peer_cert.to_cryptography()
65+
ssl_cert = {}
66+
67+
subject = tuple((('commonName', attr.value),)
68+
for attr in cert.subject.get_attributes_for_oid(NameOID.COMMON_NAME))
69+
if subject:
70+
ssl_cert['subject'] = subject
71+
72+
subject_alt_names = []
73+
try:
74+
san = cert.extensions.get_extension_for_class(x509.SubjectAlternativeName).value
75+
except x509.ExtensionNotFound:
76+
pass
77+
else:
78+
subject_alt_names.extend(('DNS', name) for name in san.get_values_for_type(x509.DNSName))
79+
subject_alt_names.extend(('IP Address', str(address))
80+
for address in san.get_values_for_type(x509.IPAddress))
81+
82+
if subject_alt_names:
83+
ssl_cert['subjectAltName'] = tuple(subject_alt_names)
84+
85+
return ssl_cert
86+
87+
88+
def _validate_pyopenssl_hostname(peer_cert, hostname):
89+
cert = _pyopenssl_cert_to_ssl_cert(peer_cert)
90+
try:
91+
host_ip = ipaddress.ip_address(hostname)
92+
except ValueError:
93+
host_ip = None
94+
95+
names = []
96+
for key, value in cert.get('subjectAltName', ()):
97+
if key == 'DNS':
98+
if host_ip is None and _dnsname_match(value, hostname):
99+
return
100+
names.append(value)
101+
elif key == 'IP Address':
102+
if host_ip is not None and _ipaddress_match(value, host_ip):
103+
return
104+
names.append(value)
105+
106+
if not names:
107+
for subject in cert.get('subject', ()):
108+
for key, value in subject:
109+
if key == 'commonName':
110+
if _dnsname_match(value, hostname):
111+
return
112+
names.append(value)
113+
114+
if len(names) > 1:
115+
raise ssl.CertificateError("hostname {!r} doesn't match either of {}".format(
116+
hostname, ', '.join(map(repr, names))))
117+
if len(names) == 1:
118+
raise ssl.CertificateError("hostname {!r} doesn't match {!r}".format(hostname, names[0]))
119+
raise ssl.CertificateError("no appropriate commonName or subjectAltName fields were found")
120+
121+
122+
def _dnsname_match(pattern, hostname):
123+
if not pattern:
124+
return False
125+
126+
wildcards = pattern.count('*')
127+
if not wildcards:
128+
return pattern.lower() == hostname.lower()
129+
if wildcards > 1:
130+
raise ssl.CertificateError(
131+
"too many wildcards in certificate DNS name: {!r}.".format(pattern))
132+
133+
leftmost, sep, remainder = pattern.partition('.')
134+
if '*' in remainder:
135+
raise ssl.CertificateError(
136+
"wildcard can only be present in the leftmost label: {!r}.".format(pattern))
137+
if not sep:
138+
raise ssl.CertificateError(
139+
"sole wildcard without additional labels is not supported: {!r}.".format(pattern))
140+
if leftmost != '*':
141+
raise ssl.CertificateError(
142+
"partial wildcards in leftmost label are not supported: {!r}.".format(pattern))
143+
144+
hostname_leftmost, sep, hostname_remainder = hostname.partition('.')
145+
if not hostname_leftmost or not sep:
146+
return False
147+
return remainder.lower() == hostname_remainder.lower()
148+
149+
150+
def _ipaddress_match(pattern, host_ip):
151+
try:
152+
return ipaddress.ip_address(pattern) == host_ip
153+
except ValueError:
154+
return False
155+
156+
59157
segment_codec_no_compression = SegmentCodec()
60158
segment_codec_lz4 = None
61159

cassandra/io/eventletreactor.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
from threading import Event
2424
import time
2525

26-
from cassandra.connection import Connection, ConnectionShutdown, Timer, TimerManager
26+
from cassandra.connection import (Connection, ConnectionShutdown, Timer, TimerManager,
27+
_validate_pyopenssl_hostname)
2728
try:
2829
from eventlet.green.OpenSSL import SSL
2930
_PYOPENSSL = True
@@ -152,10 +153,8 @@ def _validate_hostname(self):
152153
if self.uses_legacy_ssl_options:
153154
super(EventletConnection, self)._validate_hostname()
154155
else:
155-
cert_name = self._socket.get_peer_certificate().get_subject().commonName
156-
if cert_name != self.endpoint.address:
157-
raise Exception("Hostname verification failed! Certificate name '{}' "
158-
"doesn't endpoint '{}'".format(cert_name, self.endpoint.address))
156+
expected_name = (self.ssl_options or {}).get('server_hostname') or self.endpoint.address
157+
_validate_pyopenssl_hostname(self._socket.get_peer_certificate(), expected_name)
159158

160159
def _build_ssl_context_from_options(self):
161160
_check_pyopenssl()

cassandra/io/twistedreactor.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@
2828
from twisted.python.failure import Failure
2929
from zope.interface import implementer
3030

31-
from cassandra.connection import Connection, ConnectionShutdown, Timer, TimerManager, ConnectionException
31+
from cassandra.connection import (Connection, ConnectionShutdown, Timer, TimerManager, ConnectionException,
32+
_validate_pyopenssl_hostname)
3233

3334
try:
3435
from OpenSSL import SSL
@@ -192,11 +193,17 @@ def __init__(self, endpoint, ssl_context, ssl_options, check_hostname, timeout):
192193
def verify_callback(self, connection, x509, errnum, errdepth, ok):
193194
return ok
194195

196+
def _hostname_to_verify(self):
197+
return self.ssl_options.get("server_hostname") or self.endpoint.address
198+
195199
def info_callback(self, connection, where, ret):
196200
if where & SSL.SSL_CB_HANDSHAKE_DONE:
197-
if self.check_hostname and self.endpoint.address != connection.get_peer_certificate().get_subject().commonName:
198-
transport = connection.get_app_data()
199-
transport.failVerification(Failure(ConnectionException("Hostname verification failed", self.endpoint)))
201+
if self.check_hostname:
202+
try:
203+
_validate_pyopenssl_hostname(connection.get_peer_certificate(), self._hostname_to_verify())
204+
except Exception as exc:
205+
transport = connection.get_app_data()
206+
transport.failVerification(Failure(ConnectionException(str(exc), self.endpoint)))
200207

201208
def clientConnectionForTLS(self, tlsProtocol):
202209
connection = SSL.Connection(self.context, None)

tests/unit/io/test_eventletreactor.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,22 @@
1313
# limitations under the License.
1414

1515
import os
16+
import ssl
1617
import unittest
18+
from datetime import datetime, timedelta, timezone
1719
from types import SimpleNamespace
1820

1921
from unittest.mock import Mock, patch
2022

23+
try:
24+
from cryptography import x509
25+
from cryptography.hazmat.primitives import hashes
26+
from cryptography.hazmat.primitives.asymmetric import rsa
27+
from cryptography.x509.oid import NameOID
28+
from OpenSSL import crypto
29+
except ImportError:
30+
crypto = None
31+
2132
from tests.unit.io.utils import TimerTestMixin
2233
from tests import notpypy, EVENT_LOOP_MANAGER
2334

@@ -36,6 +47,27 @@
3647
os.path.dirname(__file__), '..', '..', 'integration', 'long', 'ssl', 'rootCa.crt'))
3748

3849

50+
def _make_certificate(common_name, san_dns_names=None):
51+
key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
52+
subject = issuer = x509.Name([
53+
x509.NameAttribute(NameOID.COMMON_NAME, common_name),
54+
])
55+
now = datetime.now(timezone.utc)
56+
builder = (x509.CertificateBuilder()
57+
.subject_name(subject)
58+
.issuer_name(issuer)
59+
.public_key(key.public_key())
60+
.serial_number(x509.random_serial_number())
61+
.not_valid_before(now - timedelta(days=1))
62+
.not_valid_after(now + timedelta(days=1)))
63+
if san_dns_names:
64+
builder = builder.add_extension(
65+
x509.SubjectAlternativeName([x509.DNSName(name) for name in san_dns_names]),
66+
critical=False)
67+
68+
return crypto.X509.from_cryptography(builder.sign(key, hashes.SHA256()))
69+
70+
3971
@unittest.skipIf(EventletConnection is None, "eventlet is not available")
4072
@unittest.skipIf(not getattr(eventletreactor, '_PYOPENSSL', False), "pyOpenSSL is not available")
4173
class EventletSSLContextTest(unittest.TestCase):
@@ -88,6 +120,48 @@ def test_check_hostname_option_enables_hostname_validation(self):
88120

89121
assert conn._check_hostname
90122

123+
def test_validate_hostname_rejects_mismatch(self):
124+
conn = EventletConnection.__new__(EventletConnection)
125+
conn.uses_legacy_ssl_options = False
126+
conn.endpoint = DefaultEndPoint('1.2.3.4')
127+
conn.ssl_options = {}
128+
conn._socket = Mock()
129+
conn._socket.get_peer_certificate.return_value = _make_certificate('wrong.host')
130+
131+
with self.assertRaises(ssl.CertificateError):
132+
conn._validate_hostname()
133+
134+
def test_validate_hostname_uses_server_hostname(self):
135+
conn = EventletConnection.__new__(EventletConnection)
136+
conn.uses_legacy_ssl_options = False
137+
conn.endpoint = DefaultEndPoint('proxy.host')
138+
conn.ssl_options = {'server_hostname': 'sni.host'}
139+
conn._socket = Mock()
140+
conn._socket.get_peer_certificate.return_value = _make_certificate('proxy.host', ['sni.host'])
141+
142+
conn._validate_hostname()
143+
144+
def test_validate_hostname_prefers_san_over_common_name(self):
145+
conn = EventletConnection.__new__(EventletConnection)
146+
conn.uses_legacy_ssl_options = False
147+
conn.endpoint = DefaultEndPoint('sni.host')
148+
conn.ssl_options = {}
149+
conn._socket = Mock()
150+
conn._socket.get_peer_certificate.return_value = _make_certificate('sni.host', ['other.host'])
151+
152+
with self.assertRaises(ssl.CertificateError):
153+
conn._validate_hostname()
154+
155+
def test_validate_hostname_matches_wildcard_san(self):
156+
conn = EventletConnection.__new__(EventletConnection)
157+
conn.uses_legacy_ssl_options = False
158+
conn.endpoint = DefaultEndPoint('node.example.com')
159+
conn.ssl_options = {}
160+
conn._socket = Mock()
161+
conn._socket.get_peer_certificate.return_value = _make_certificate('other.host', ['*.example.com'])
162+
163+
conn._validate_hostname()
164+
91165
def test_wrap_socket_from_context_returns_wrapped_socket(self):
92166
conn = EventletConnection.__new__(EventletConnection)
93167
conn.ssl_context = object()

tests/unit/io/test_twistedreactor.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,19 @@
1414

1515
import os
1616
import unittest
17+
from datetime import datetime, timedelta, timezone
1718
from types import SimpleNamespace
1819
from unittest.mock import Mock, patch
1920

21+
try:
22+
from cryptography import x509
23+
from cryptography.hazmat.primitives import hashes
24+
from cryptography.hazmat.primitives.asymmetric import rsa
25+
from cryptography.x509.oid import NameOID
26+
from OpenSSL import crypto
27+
except ImportError:
28+
crypto = None
29+
2030
from cassandra.connection import Connection, DefaultEndPoint
2131

2232
try:
@@ -35,6 +45,27 @@
3545
os.path.dirname(__file__), '..', '..', 'integration', 'long', 'ssl', 'rootCa.crt'))
3646

3747

48+
def _make_certificate(common_name, san_dns_names=None):
49+
key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
50+
subject = issuer = x509.Name([
51+
x509.NameAttribute(NameOID.COMMON_NAME, common_name),
52+
])
53+
now = datetime.now(timezone.utc)
54+
builder = (x509.CertificateBuilder()
55+
.subject_name(subject)
56+
.issuer_name(issuer)
57+
.public_key(key.public_key())
58+
.serial_number(x509.random_serial_number())
59+
.not_valid_before(now - timedelta(days=1))
60+
.not_valid_after(now + timedelta(days=1)))
61+
if san_dns_names:
62+
builder = builder.add_extension(
63+
x509.SubjectAlternativeName([x509.DNSName(name) for name in san_dns_names]),
64+
critical=False)
65+
66+
return crypto.X509.from_cryptography(builder.sign(key, hashes.SHA256()))
67+
68+
3869
@unittest.skipIf(TwistedConnection is None, "Twisted libraries are not available")
3970
@unittest.skipIf(not getattr(twistedreactor, '_HAS_SSL', False), "pyOpenSSL is not available")
4071
class TwistedSSLContextTest(unittest.TestCase):
@@ -77,6 +108,39 @@ def test_check_hostname_option_enables_hostname_validation(self):
77108

78109
assert conn._check_hostname
79110

111+
def test_hostname_verification_uses_server_hostname(self):
112+
context = Mock()
113+
creator = twistedreactor._SSLCreator(DefaultEndPoint('proxy.host'), context,
114+
{'server_hostname': 'sni.host'}, True, None)
115+
connection = Mock()
116+
connection.get_peer_certificate.return_value = _make_certificate('proxy.host', ['sni.host'])
117+
118+
creator.info_callback(connection, twistedreactor.SSL.SSL_CB_HANDSHAKE_DONE, None)
119+
120+
connection.get_app_data.assert_not_called()
121+
122+
def test_hostname_verification_prefers_san_over_common_name(self):
123+
context = Mock()
124+
creator = twistedreactor._SSLCreator(DefaultEndPoint('sni.host'), context, {}, True, None)
125+
connection = Mock()
126+
transport = Mock()
127+
connection.get_app_data.return_value = transport
128+
connection.get_peer_certificate.return_value = _make_certificate('sni.host', ['other.host'])
129+
130+
creator.info_callback(connection, twistedreactor.SSL.SSL_CB_HANDSHAKE_DONE, None)
131+
132+
transport.failVerification.assert_called_once()
133+
134+
def test_hostname_verification_matches_wildcard_san(self):
135+
context = Mock()
136+
creator = twistedreactor._SSLCreator(DefaultEndPoint('node.example.com'), context, {}, True, None)
137+
connection = Mock()
138+
connection.get_peer_certificate.return_value = _make_certificate('other.host', ['*.example.com'])
139+
140+
creator.info_callback(connection, twistedreactor.SSL.SSL_CB_HANDSHAKE_DONE, None)
141+
142+
connection.get_app_data.assert_not_called()
143+
80144

81145
class TestTwistedTimer(TimerTestMixin, unittest.TestCase):
82146
"""

0 commit comments

Comments
 (0)