File CVE-2023-40217-avoid-ssl-pre-close.patch of Package python

Overview Repositories Revisions Requests Users Attributes Meta

File CVE-2023-40217-avoid-ssl-pre-close.patch of Package python (Revision 8a6af07ab5564221f9f37d634589b0fb)

Currently displaying revision 8a6af07ab5564221f9f37d634589b0fb , Show latest

From f0c1e55dfd28970196768a6997a6dc0eab0f5259 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Langa?= <lukasz@langa.pl>
Date: Tue, 22 Aug 2023 17:39:17 +0200
Subject: [PATCH] gh-108310: Fix CVE-2023-40217: Check for & avoid the ssl
 pre-close flaw
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

Co-Authored-By: Gregory P. Smith [Google LLC] <greg@krypto.org>
---
 Lib/ssl.py                                    |  31 ++-
 Lib/test/test_ssl.py                          | 215 ++++++++++++++++++
 ...-08-22-17-39-12.gh-issue-108310.fVM3sg.rst |   7 +
 3 files changed, 252 insertions(+), 1 deletion(-)
 create mode 100644 Misc/NEWS.d/next/Security/2023-08-22-17-39-12.gh-issue-108310.fVM3sg.rst

Index: Python-2.7.18/Lib/ssl.py
===================================================================
--- Python-2.7.18.orig/Lib/ssl.py
+++ Python-2.7.18/Lib/ssl.py
@@ -576,10 +576,13 @@ class SSLSocket(socket):
                              "in client mode")
         if self._context.check_hostname and not server_hostname:
             raise ValueError("check_hostname requires server_hostname")
+        self._closed = False
+        self._sslobj = None
         self.server_side = server_side
         self.server_hostname = server_hostname
         self.do_handshake_on_connect = do_handshake_on_connect
         self.suppress_ragged_eofs = suppress_ragged_eofs
+        sock_timeout = sock.gettimeout()
 
         # See if we are connected
         try:
@@ -588,11 +591,38 @@ class SSLSocket(socket):
             if e.errno != errno.ENOTCONN:
                 raise
             connected = False
+            blocking = self.gettimeout() == 0
+            self.setblocking(False)
+            try:
+                # We are not connected so this is not supposed to block, but
+                # testing revealed otherwise on macOS and Windows so we do
+                # the non-blocking dance regardless. Our raise when any data
+                # is found means consuming the data is harmless.
+                notconn_pre_handshake_data = self.recv(1)
+            except socket_error as e:
+                # EINVAL occurs for recv(1) on non-connected on unix sockets.
+                if e.errno not in (errno.ENOTCONN, errno.EINVAL):
+                    raise
+                notconn_pre_handshake_data = b''
+            self.setblocking(blocking)
+            if notconn_pre_handshake_data:
+                # This prevents pending data sent to the socket before it was
+                # closed from escaping to the caller who could otherwise
+                # presume it came through a successful TLS connection.
+                reason = "Closed before TLS handshake with data in recv buffer."
+                notconn_pre_handshake_data_error = SSLError(e.errno, reason)
+                # Add the SSLError attributes that _ssl.c always adds.
+                notconn_pre_handshake_data_error.reason = reason
+                notconn_pre_handshake_data_error.library = None
+                try:
+                    self.close()
+                except socket_error:
+                    pass
+                raise notconn_pre_handshake_data_error
         else:
             connected = True
 
-        self._closed = False
-        self._sslobj = None
+        self.settimeout(sock_timeout)  # Must come after setblocking() calls.
         self._connected = connected
         if connected:
             # create the SSL object
Index: Python-2.7.18/Lib/test/test_ssl.py
===================================================================
--- Python-2.7.18.orig/Lib/test/test_ssl.py
+++ Python-2.7.18/Lib/test/test_ssl.py
@@ -20,6 +20,8 @@ import traceback
 import weakref
 import platform
 import re
+import struct
+import httplib
 import functools
 from contextlib import closing
 
@@ -3262,6 +3264,217 @@ else:
                 self.assertRaises(ValueError, s.write, b'hello')
 
 
+def set_socket_so_linger_on_with_zero_timeout(sock):
+    sock.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack('ii', 1, 0))
+
+
+class TestPreHandshakeClose(unittest.TestCase):
+    """Verify behavior of close sockets with received data before to the handshake.
+    """
+
+    class SingleConnectionTestServerThread(threading.Thread):
+
+        def __init__(self, name=None, call_after_accept=None):
+            self.call_after_accept = call_after_accept
+            self.received_data = b''  # set by .run()
+            self.wrap_error = None  # set by .run()
+            self.listener = None  # set by .start()
+            self.port = None  # set by .start()
+            super().__init__(name=name)
+
+        def __enter__(self):
+            self.start()
+            return self
+
+        def __exit__(self, *args):
+            try:
+                if self.listener:
+                    self.listener.close()
+            except OSError:
+                pass
+            self.join()
+            self.wrap_error = None  # avoid dangling references
+
+        def start(self):
+            self.ssl_ctx = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH)
+            self.ssl_ctx.verify_mode = ssl.CERT_REQUIRED
+            self.ssl_ctx.load_verify_locations(cafile=ONLYCERT)
+            self.ssl_ctx.load_cert_chain(certfile=ONLYCERT, keyfile=ONLYKEY)
+            self.listener = socket.socket()
+            self.port = support.bind_port(self.listener)
+            self.listener.settimeout(2.0)
+            self.listener.listen(1)
+            super().start()
+
+        def run(self):
+            conn, address = self.listener.accept()
+            self.listener.close()
+            with conn:
+                if self.call_after_accept(conn):
+                    return
+                try:
+                    tls_socket = self.ssl_ctx.wrap_socket(conn, server_side=True)
+                except OSError as err:  # ssl.SSLError inherits from OSError
+                    self.wrap_error = err
+                else:
+                    try:
+                        self.received_data = tls_socket.recv(400)
+                    except OSError:
+                        pass  # closed, protocol error, etc.
+
+    def non_linux_skip_if_other_okay_error(self, err):
+        if sys.platform == "linux":
+            return  # Expect the full test setup to always work on Linux.
+        if (isinstance(err, ConnectionResetError) or
+            (isinstance(err, OSError) and err.errno == errno.EINVAL) or
+            re.search('wrong.version.number', getattr(err, "reason", ""), re.I)):
+            # On Windows the TCP RST leads to a ConnectionResetError
+            # (ECONNRESET) which Linux doesn't appear to surface to userspace.
+            # If wrap_socket() winds up on the "if connected:" path and doing
+            # the actual wrapping... we get an SSLError from OpenSSL. Typically
+            # WRONG_VERSION_NUMBER. While appropriate, neither is the scenario
+            # we're specifically trying to test. The way this test is written
+            # is known to work on Linux. We'll skip it anywhere else that it
+            # does not present as doing so.
+            self.skipTest("Could not recreate conditions on %s: %s" % (sys.platform, err))
+        # If maintaining this conditional winds up being a problem.
+        # just turn this into an unconditional skip anything but Linux.
+        # The important thing is that our CI has the logic covered.
+
+    def test_preauth_data_to_tls_server(self):
+        server_accept_called = threading.Event()
+        ready_for_server_wrap_socket = threading.Event()
+
+        def call_after_accept(unused):
+            server_accept_called.set()
+            if not ready_for_server_wrap_socket.wait(2.0):
+                raise RuntimeError("wrap_socket event never set, test may fail.")
+            return False  # Tell the server thread to continue.
+
+        server = self.SingleConnectionTestServerThread(
+                call_after_accept=call_after_accept,
+                name="preauth_data_to_tls_server")
+        server.__enter__()  # starts it
+        self.addCleanup(server.__exit__)  # ... & unittest.TestCase stops it.
+
+        with socket.socket() as client:
+            client.connect(server.listener.getsockname())
+            # This forces an immediate connection close via RST on .close().
+            set_socket_so_linger_on_with_zero_timeout(client)
+            client.setblocking(False)
+
+            server_accept_called.wait()
+            client.send(b"DELETE /data HTTP/1.0\r\n\r\n")
+            client.close()  # RST
+
+        ready_for_server_wrap_socket.set()
+        server.join()
+        wrap_error = server.wrap_error
+        self.assertEqual(b"", server.received_data)
+        self.assertIsInstance(wrap_error, OSError)  # All platforms.
+        self.non_linux_skip_if_other_okay_error(wrap_error)
+        self.assertIsInstance(wrap_error, ssl.SSLError)
+        self.assertIn("before TLS handshake with data", wrap_error.args[1])
+        self.assertIn("before TLS handshake with data", wrap_error.reason)
+        self.assertNotEqual(0, wrap_error.args[0])
+        self.assertIsNone(wrap_error.library, msg="attr must exist")
+
+    def test_preauth_data_to_tls_client(self):
+        client_can_continue_with_wrap_socket = threading.Event()
+
+        def call_after_accept(conn_to_client):
+            # This forces an immediate connection close via RST on .close().
+            set_socket_so_linger_on_with_zero_timeout(conn_to_client)
+            conn_to_client.send(
+                    b"HTTP/1.0 307 Temporary Redirect\r\n"
+                    b"Location: https://example.com/someone-elses-server\r\n"
+                    b"\r\n")
+            conn_to_client.close()  # RST
+            client_can_continue_with_wrap_socket.set()
+            return True  # Tell the server to stop.
+
+        server = self.SingleConnectionTestServerThread(
+                call_after_accept=call_after_accept,
+                name="preauth_data_to_tls_client")
+        server.__enter__()  # starts it
+        self.addCleanup(server.__exit__)  # ... & unittest.TestCase stops it.
+
+        # Redundant; call_after_accept sets SO_LINGER on the accepted conn.
+        set_socket_so_linger_on_with_zero_timeout(server.listener)
+
+        with socket.socket() as client:
+            client.connect(server.listener.getsockname())
+            if not client_can_continue_with_wrap_socket.wait(2.0):
+                self.fail("test server took too long.")
+            ssl_ctx = ssl.create_default_context()
+            try:
+                tls_client = ssl_ctx.wrap_socket(
+                        client, server_hostname="localhost")
+            except OSError as err:  # SSLError inherits from OSError
+                wrap_error = err
+                received_data = b""
+            else:
+                wrap_error = None
+                received_data = tls_client.recv(400)
+                tls_client.close()
+
+        server.join()
+        self.assertEqual(b"", received_data)
+        self.assertIsInstance(wrap_error, OSError)  # All platforms.
+        self.non_linux_skip_if_other_okay_error(wrap_error)
+        self.assertIsInstance(wrap_error, ssl.SSLError)
+        self.assertIn("before TLS handshake with data", wrap_error.args[1])
+        self.assertIn("before TLS handshake with data", wrap_error.reason)
+        self.assertNotEqual(0, wrap_error.args[0])
+        self.assertIsNone(wrap_error.library, msg="attr must exist")
+
+    def test_https_client_non_tls_response_ignored(self):
+
+        server_responding = threading.Event()
+
+        class SynchronizedHTTPSConnection(httplib.HTTPSConnection):
+            def connect(self):
+                httplib.HTTPConnection.connect(self)
+                # Wait for our fault injection server to have done its thing.
+                if not server_responding.wait(1.0) and support.verbose:
+                    sys.stdout.write("server_responding event never set.")
+                self.sock = self._context.wrap_socket(
+                        self.sock, server_hostname=self.host)
+
+        def call_after_accept(conn_to_client):
+            # This forces an immediate connection close via RST on .close().
+            set_socket_so_linger_on_with_zero_timeout(conn_to_client)
+            conn_to_client.send(
+                    b"HTTP/1.0 402 Payment Required\r\n"
+                    b"\r\n")
+            conn_to_client.close()  # RST
+            server_responding.set()
+            return True  # Tell the server to stop.
+
+        server = self.SingleConnectionTestServerThread(
+                call_after_accept=call_after_accept,
+                name="non_tls_http_RST_responder")
+        server.__enter__()  # starts it
+        self.addCleanup(server.__exit__)  # ... & unittest.TestCase stops it.
+        # Redundant; call_after_accept sets SO_LINGER on the accepted conn.
+        set_socket_so_linger_on_with_zero_timeout(server.listener)
+
+        connection = SynchronizedHTTPSConnection(
+                "localhost",
+                port=server.port,
+                context=ssl.create_default_context(),
+                timeout=2.0,
+        )
+        # There are lots of reasons this raises as desired, long before this
+        # test was added. Sending the request requires a successful TLS wrapped
+        # socket; that fails if the connection is broken. It may seem pointless
+        # to test this. It serves as an illustration of something that we never
+        # want to happen... properly not happening.
+        with self.assertRaises(OSError) as err_ctx:
+            connection.request("HEAD", "/test", headers={"Host": "localhost"})
+            response = connection.getresponse()
+
+
 def test_main(verbose=False):
     if support.verbose:
         plats = {
Index: Python-2.7.18/Misc/NEWS.d/next/Security/2023-08-22-17-39-12.gh-issue-108310.fVM3sg.rst
===================================================================
--- /dev/null
+++ Python-2.7.18/Misc/NEWS.d/next/Security/2023-08-22-17-39-12.gh-issue-108310.fVM3sg.rst
@@ -0,0 +1,7 @@
+Fixed an issue where instances of :class:`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.  Security issue reported as
+`CVE-2023-40217
+<https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-40217>`_ by
+Aapo Oksman. Patch by Gregory P. Smith.

Places

File CVE-2023-40217-avoid-ssl-pre-close.patch of Package python (Revision 8a6af07ab5564221f9f37d634589b0fb)

Places