8000 [CVE-2023-40217] Check for & avoid the ssl pre-close flaw · openSUSE-Python/cpython@1c99fc6 · GitHub
[go: up one dir, main page]
Skip to content
8000
8000

Commit 1c99fc6

Browse files
mceplmcepl
committed
[CVE-2023-40217] Check for & avoid the ssl pre-close flaw
Instances of `ssl.SSLSocket` were vulnerable to a bypass of the TLS handshake and included protections (like certificate verification) and treating sent unencrypted data as if it were post-handshake TLS encrypted data. The vulnerability is caused when a socket is connected, data is sent by the malicious peer and stored in a buffer, and then the malicious peer closes the socket within a small timing window before the other peers’ TLS handshake can begin. After this sequence of events the closed socket will not immediately attempt a TLS handshake due to not being connected but will also allow the buffered data to be read as if a successful TLS handshake had occurred. Code is from gh#python/cpython@b4bcc06, it was released upstream in 3.8.18. Fixes: bsc#1214692 Fixes: gh#python#108315 Patch: CVE-2023-40217-avoid-ssl-pre-close.patch
1 parent e102d59 commit 1c99fc6

File tree

3 files changed

+254
-3
lines changed

3 files changed

+254
-3
lines changed

‎Lib/ssl.py

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,8 @@ def __init__(self, sock=None, keyfile=None, certfile=None,
771771
"client mode")
772772
if self._context.check_hostname and not server_hostname:
773773
raise ValueError("check_hostname requires server_hostname")
774+
self._closed = False
775+
self._sslobj = None
774776
self._session = _session
775777
self.server_side = server_side
776778
self.server_hostname = server_hostname
@@ -782,7 +784,7 @@ def __init__(self, sock=None, keyfile=None, certfile=None,
782784
type=sock.type,
783785
proto=sock.proto,
784786
fileno=sock.fileno())
785-
self.settimeout(sock.gettimeout())
787+
sock_timeout = sock.gettimeout()
786788
sock.detach()
787789
elif fileno is not None:
788790
socket.__init__(self, fileno=fileno)
@@ -796,11 +798,38 @@ def __init__(self, sock=None, keyfile=None, certfile=None,
796798
if e.errno != errno.ENOTCONN:
797799
raise
798800
connected = False
801+
blocking = self.gettimeout() == 0
802+
self.setblocking(False)
803+
try:
804+
# We are not connected so this is not supposed to block, but
805+
# testing revealed otherwise on macOS and Windows so we do
806+
# the non-blocking dance regardless. Our raise when any data
807+
# is found means consuming the data is harmless.
808+
notconn_pre_handshake_data = self.recv(1)
809+
except OSError as e:
810+
# EINVAL occurs for recv(1) on non-connected on unix sockets.
811+
if e.errno not in (errno.ENOTCONN, errno.EINVAL):
812+
raise
813+
notconn_pre_handshake_data = b''
814+
self.setblocking(blocking)
815+
if notconn_pre_handshake_data:
816+
# This prevents pending data sent to the socket before it was
817+
# closed from escaping to the caller who could otherwise
818+
# presume it came through a successful TLS connection.
819+
reason = "Closed before TLS handshake with data in recv buffer."
820+
notconn_pre_handshake_data_error = SSLError(e.errno, reason)
821+
# Add the SSLError attributes that _ssl.c always adds.
822+
notconn_pre_handshake_data_error.reason = reason
823+
notconn_pre_handshake_data_error.library = None
824+
try:
825+
self.close()
826+
except OSError:
827+
pass
828+
raise notconn_pre_handshake_data_error
799829
else:
800830
connected = True
801831

802-
self._closed = False
803-
self._sslobj = None
832+
self.settimeout(sock_timeout) # Must come after setblocking() calls.
804833
self._connected = connected
805834
if connected:
806835
# create the SSL object

‎Lib/test/test_ssl.py

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33
import sys
44
import unittest
55
from test import support
6+
import re
67
import socket
78
import select
9+
import struct
810
import time
911
import datetime
1012
import gc
13+
import http.client
1114
import os
1215
import errno
1316
import pprint
@@ -3917,6 +3920,218 @@ def test_pha_not_tls13(self):
39173920
self.assertIn(b'WRONG_SSL_VERSION', s.recv(1024))
39183921

39193922

3923+
def set_socket_so_linger_on_with_zero_timeout(sock):
3924+
sock.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack('ii', 1, 0))
3925+
3926+
3927+
class TestPreHandshakeClose(unittest.TestCase):
3928+
"""Verify behavior of close sockets with received data before to the handshake.
3929+
"""
3930+
3931+
class SingleConnectionTestServerThread(threading.Thread):
3932+
3933+
def __init__(self, *, name, call_after_accept):
3934+
self.call_after_accept = call_after_accept
3935+
self.received_data = b'' # set by .run()
3936+
self.wrap_error = None # set by .run()
3937+
self.listener = None # set by .start()
3938+
self.port = None # set by .start()
3939+
super().__init__(name=name)
3940+
3941+
def __enter__(self):
3942+
self.start()
3943+
return self
3944+
3945+
def __exit__(self, *args):
3946+
try:
3947+
if self.listener:
3948+
self.listener.close()
3949+
except OSError:
3950+
pass
3951+
self.join()
3952+
self.wrap_error = None # avoid dangling references
3953+
3954+
def start(self):
3955+
self.ssl_ctx = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH)
3956+
self.ssl_ctx.verify_mode = ssl.CERT_REQUIRED
3957+
self.ssl_ctx.load_verify_locations(cafile=ONLYCERT)
3958+
self.ssl_ctx.load_cert_chain(certfile=ONLYCERT, keyfile=ONLYKEY)
3959+
self.listener = socket.socket()
3960+
self.port = support.bind_port(self.listener)
3961+
self.listener.settimeout(2.0)
3962+
self.listener.listen(1)
3963+
super().start()
3964+
3965+
def run(self):
3966+
conn, address = self.listener.accept()
3967+
self.listener.close()
3968+
with conn:
3969+
if self.call_after_accept(conn):
3970+
return
3971+
try:
3972+
tls_socket = self.ssl_ctx.wrap_socket(conn, server_side=True)
3973+
except OSError as err: # ssl.SSLError inherits from OSError
3974+
self.wrap_error = err
3975+
else:
3976+
try:
3977+
self.received_data = tls_socket.recv(400)
3978+
except OSError:
3979+
pass # closed, protocol error, etc.
3980+
3981+
def non_linux_skip_if_other_okay_error(self, err):
3982+
if sys.platform == "linux":
3983+
return # Expect the full test setup to always work on Linux.
3984+
if (isinstance(err, ConnectionResetError) or
3985+
(isinstance(err, OSError) and err.errno == errno.EINVAL) or
3986+
re.search('wrong.version.number', getattr(err, "reason", ""), re.I)):
3987+
# On Windows the TCP RST leads to a ConnectionResetError
3988+
# (ECONNRESET) which Linux doesn't appear to surface to userspace.
3989+
# If wrap_socket() winds up on the "if connected:" path and doing
3990+
# the actual wrapping... we get an SSLError from OpenSSL. Typically
3991+
# WRONG_VERSION_NUMBER. While appropriate, neither is the scenario
3992+
# we're specifically trying to test. The way this test is written
3993+
# is known to work on Linux. We'll skip it anywhere else that it
3994+
# does not present as doing so.
3995+
self.skipTest(f"Could not recreate conditions on {sys.platform}:"
3996+
f" {err}")
3997+
# If maintaining this conditional winds up being a problem.
3998+
# just turn this into an unconditional skip anything but Linux.
3999+
# The important thing is that our CI has the logic covered.
4000+
4001+
def test_preauth_data_to_tls_server(self):
4002+
server_accept_called = threading.Event()
4003+
ready_for_server_wrap_socket = threading.Event()
4004+
4005+
def call_after_accept(unused):
4006+
server_accept_called.set()
4007+
if not ready_for_server_wrap_socket.wait(2.0):
4008+
raise RuntimeError("wrap_socket event never set, test may fail.")
4009+
return False # Tell the server thread to continue.
4010+
4011+
server = self.SingleConnectionTestServerThread(
4012+
call_after_accept=call_after_accept,
4013+
name="preauth_data_to_tls_server")
4014+
server.__enter__() # starts it
4015+
self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it.
4016+
4017+
with socket.socket() as client:
4018+
client.connect(server.listener.getsockname())
4019+
# This forces an immediate connection close via RST on .close().
4020+
set_socket_so_linger_on_with_zero_timeout(client)
4021+
client.setblocking(False)
4022+
4023+
server_accept_called.wait()
4024+
client.send(b"DELETE /data HTTP/1.0\r\n\r\n")
4025+
client.close() # RST
4026+
4027+
ready_for_server_wrap_socket.set()
4028+
server.join()
4029+
wrap_error = server.wrap_error
4030+
self.assertEqual(b"", server.received_data)
4031+
self.assertIsInstance(wrap_error, OSError) # All platforms.
4032+
self.non_linux_skip_if_other_okay_error(wrap_error)
4033+
self.assertIsInstance(wrap_error, ssl.SSLError)
4034+
self.assertIn("before TLS handshake with data", wrap_error.args[1])
4035+
self.assertIn("before TLS handshake with data", wrap_error.reason)
4036+
self.assertNotEqual(0, wrap_error.args[0])
4037+
self.assertIsNone(wrap_error.library, msg="attr must exist")
4038+
4039+
def test_preauth_data_to_tls_client(self):
4040+
client_can_continue_with_wrap_socket = threading.Event()
4041+
4042+
def call_after_accept(conn_to_client):
4043+
# This forces an immediate connection close via RST on .close().
4044+
set_socket_so_linger_on_with_zero_timeout(conn_to_client)
4045+
conn_to_client.send(
4046+
b"HTTP/1.0 307 Temporary Redirect\r\n"
4047+
b"Location: https://example.com/someone-elses-server\r\n"
4048+
b"\r\n")
4049+
conn_to_client.close() # RST
4050+
client_can_continue_with_wrap_socket.set()
4051+
return True # Tell the server to stop.
4052+
4053+
server = self.SingleConnectionTestServerThread(
4054+
call_after_accept=call_after_accept,
4055+
name="preauth_data_to_tls_client")
4056+
server.__enter__() # starts it
4057+
self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it.
4058+
4059+
# Redundant; call_after_accept sets SO_LINGER on the accepted conn.
4060+
set_socket_so_linger_on_with_zero_timeout(server.listener)
4061+
4062+
with socket.socket() as client:
4063+
client.connect(server.listener.getsockname())
4064+
if not client_can_continue_with_wrap_socket.wait(2.0):
4065+
self.fail("test server took too long.")
4066+
ssl_ctx = ssl.create_default_context()
4067+
try:
4068+
tls_client = ssl_ctx.wrap_socket(
4069+
client, server_hostname="localhost")
4070+
except OSError as err: # SSLError inherits from OSError
4071+
wrap_error = err
4072+
received_data = b""
4073+
else:
4074+
wrap_error = None
4075+
received_data = tls_client.recv(400)
4076+
tls_client.close()
4077+
4078+
server.join()
4079+
self.assertEqual(b"", received_data)
4080+
self.assertIsInstance(wrap_error, OSError) # All platforms.
4081+
self.non_linux_skip_if_other_okay_error(wrap_error)
4082+
self.assertIsInstance(wrap_error, ssl.SSLError)
4083+
self.assertIn("before TLS handshake with data", wrap_error.args[1])
4084+
self.assertIn("before TLS handshake with data", wrap_error.reason)
4085+
self.assertNotEqual(0, wrap_error.args[0])
4086+
self.assertIsNone(wrap_error.library, msg="attr must exist")
4087+
4088+
def test_https_client_non_tls_response_ignored(self):
4089+
4090+
server_responding = threading.Event()
4091+
4092+
class SynchronizedHTTPSConnection(http.client.HTTPSConnection):
4093+
def connect(self):
4094+
http.client.HTTPConnection.connect(self)
4095+
# Wait for our fault injection server to have done its thing.
4096+
if not server_responding.wait(1.0) and support.verbose:
4097+
sys.stdout.write("server_responding event never set.")
4098+
self.sock = self._context.wrap_socket(
4099+
self.sock, server_hostname=self.host)
4100+
4101+
def call_after_accept(conn_to_client):
4102+
# This forces an immediate connection close via RST on .close().
4103+
set_socket_so_linger_on_with_zero_timeout(conn_to_client)
4104+
conn_to_client.send(
4105+
b"HTTP/1.0 402 Payment Required\r\n"
4106+
b"\r\n")
4107+
conn_to_client.close() # RST
4108+
server_responding.set()
4109+
return True # Tell the server to stop.
4110+
4111+
server = self.SingleConnectionTestServerThread(
4112+
call_after_accept=call_after_accept,
4113+
name="non_tls_http_RST_responder")
4114+
server.__enter__() # starts it
4115+
self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it.
4116+
# Redundant; call_after_accept sets SO_LINGER on the accepted conn.
4117+
set_socket_so_linger_on_with_zero_timeout(server.listener)
4118+
4119+
connection = SynchronizedHTTPSConnection(
4120+
f"localhost",
4121+
port=server.port,
4122+
context=ssl.create_default_context(),
4123+
timeout=2.0,
4124+
)
4125+
# There are lots of reasons this raises as desired, long before this
4126+
# test was added. Sending the request requires a successful TLS wrapped
4127+
# socket; that fails if the connection is broken. It may seem pointless
4128+
# to test this. It serves as an illustration of something that we never
4129+
# want to happen... properly not happening.
4130+
with self.assertRaises(OSError) as err_ctx:
4131+
connection.request("HEAD", "/test", headers={"Host": "localhost"})
4132+
response = connection.getresponse()
4133+
4134+
39204135
def test_main(verbose=False):
39214136
if support.verbose:
39224137
import warnings

‎Misc/NEWS.d/next/Security/2023-08-22-17-39-12.gh-issue-108310.fVM3sg.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Fixed an issue where instances of :class:`ssl.SSLSocket` were vulnerable to
2+
a bypass of the TLS handshake and included protections (like certificate
3+
verification) and treating sent unencrypted data as if it were
4+
post-handshake TLS encrypted data. Security issue reported as
5+
`CVE-2023-40217
6+
<https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-40217>`_ by
7+
Aapo Oksman. Patch by Gregory P. Smith.

0 commit comments

Comments
 (0)
0