|
| 1 | +From f0c1e55dfd28970196768a6997a6dc0eab0f5259 Mon Sep 17 00:00:00 2001 |
| 2 | +From: =?UTF-8?q?=C5=81ukasz=20Langa?= <lukasz@langa.pl> |
| 3 | +Date: Tue, 22 Aug 2023 17:39:17 +0200 |
| 4 | +Subject: [PATCH] gh-108310: Fix CVE-2023-40217: Check for & avoid the ssl |
| 5 | + pre-close flaw |
| 6 | +MIME-Version: 1.0 |
| 7 | +Content-Type: text/plain; charset=UTF-8 |
| 8 | +Content-Transfer-Encoding: 8bit |
| 9 | + |
| 10 | +Instances of `ssl.SSLSocket` were vulnerable to a bypass of the TLS handshake |
| 11 | +and included protections (like certificate verification) and treating sent |
| 12 | +unencrypted data as if it were post-handshake TLS encrypted data. |
| 13 | + |
| 14 | +The vulnerability is caused when a socket is connected, data is sent by the |
| 15 | +malicious peer and stored in a buffer, and then the malicious peer closes the |
| 16 | +socket within a small timing window before the other peers’ TLS handshake can |
| 17 | +begin. After this sequence of events the closed socket will not immediately |
| 18 | +attempt a TLS handshake due to not being connected but will also allow the |
| 19 | +buffered data to be read as if a successful TLS handshake had occurred. |
| 20 | + |
| 21 | +Co-Authored-By: Gregory P. Smith [Google LLC] <greg@krypto.org> |
| 22 | +--- |
| 23 | + Lib/ssl.py | 31 ++- |
| 24 | + Lib/test/test_ssl.py | 215 ++++++++++++++++++ |
| 25 | + ...-08-22-17-39-12.gh-issue-108310.fVM3sg.rst | 7 + |
| 26 | + 3 files changed, 252 insertions(+), 1 deletion(-) |
| 27 | + create mode 100644 Misc/NEWS.d/next/Security/2023-08-22-17-39-12.gh-issue-108310.fVM3sg.rst |
| 28 | + |
| 29 | +Index: Python-2.7.18/Lib/ssl.py |
| 30 | +=================================================================== |
| 31 | +--- Python-2.7.18.orig/Lib/ssl.py |
| 32 | ++++ Python-2.7.18/Lib/ssl.py |
| 33 | +@@ -576,10 +576,13 @@ class SSLSocket(socket): |
| 34 | + "in client mode") |
| 35 | + if self._context.check_hostname and not server_hostname: |
| 36 | + raise ValueError("check_hostname requires server_hostname") |
| 37 | ++ self._closed = False |
| 38 | ++ self._sslobj = None |
| 39 | + self.server_side = server_side |
| 40 | + self.server_hostname = server_hostname |
| 41 | + self.do_handshake_on_connect = do_handshake_on_connect |
| 42 | + self.suppress_ragged_eofs = suppress_ragged_eofs |
| 43 | ++ sock_timeout = sock.gettimeout() |
| 44 | + |
| 45 | + # See if we are connected |
| 46 | + try: |
| 47 | +@@ -588,11 +591,38 @@ class SSLSocket(socket): |
| 48 | + if e.errno != errno.ENOTCONN: |
| 49 | + raise |
| 50 | + connected = False |
| 51 | ++ blocking = self.gettimeout() == 0 |
| 52 | ++ self.setblocking(False) |
| 53 | ++ try: |
| 54 | ++ # We are not connected so this is not supposed to block, but |
| 55 | ++ # testing revealed otherwise on macOS and Windows so we do |
| 56 | ++ # the non-blocking dance regardless. Our raise when any data |
| 57 | ++ # is found means consuming the data is harmless. |
| 58 | ++ notconn_pre_handshake_data = self.recv(1) |
| 59 | ++ except socket_error as e: |
| 60 | ++ # EINVAL occurs for recv(1) on non-connected on unix sockets. |
| 61 | ++ if e.errno not in (errno.ENOTCONN, errno.EINVAL): |
| 62 | ++ raise |
| 63 | ++ notconn_pre_handshake_data = b'' |
| 64 | ++ self.setblocking(blocking) |
| 65 | ++ if notconn_pre_handshake_data: |
| 66 | ++ # This prevents pending data sent to the socket before it was |
| 67 | ++ # closed from escaping to the caller who could otherwise |
| 68 | ++ # presume it came through a successful TLS connection. |
| 69 | ++ reason = "Closed before TLS handshake with data in recv buffer." |
| 70 | ++ notconn_pre_handshake_data_error = SSLError(e.errno, reason) |
| 71 | ++ # Add the SSLError attributes that _ssl.c always adds. |
| 72 | ++ notconn_pre_handshake_data_error.reason = reason |
| 73 | ++ notconn_pre_handshake_data_error.library = None |
| 74 | ++ try: |
| 75 | ++ self.close() |
| 76 | ++ except socket_error: |
| 77 | ++ pass |
| 78 | ++ raise notconn_pre_handshake_data_error |
| 79 | + else: |
| 80 | + connected = True |
| 81 | + |
| 82 | +- self._closed = False |
| 83 | +- self._sslobj = None |
| 84 | ++ self.settimeout(sock_timeout) # Must come after setblocking() calls. |
| 85 | + self._connected = connected |
| 86 | + if connected: |
| 87 | + # create the SSL object |
| 88 | +Index: Python-2.7.18/Lib/test/test_ssl.py |
| 89 | +=================================================================== |
| 90 | +--- Python-2.7.18.orig/Lib/test/test_ssl.py |
| 91 | ++++ Python-2.7.18/Lib/test/test_ssl.py |
| 92 | +@@ -20,6 +20,8 @@ import traceback |
| 93 | + import weakref |
| 94 | + import platform |
| 95 | + import re |
| 96 | ++import struct |
| 97 | ++import httplib |
| 98 | + import functools |
| 99 | + from contextlib import closing |
| 100 | + |
| 101 | +@@ -3262,6 +3264,217 @@ else: |
| 102 | + self.assertRaises(ValueError, s.write, b'hello') |
| 103 | + |
| 104 | + |
| 105 | ++def set_socket_so_linger_on_with_zero_timeout(sock): |
| 106 | ++ sock.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack('ii', 1, 0)) |
| 107 | ++ |
| 108 | ++ |
| 109 | ++class TestPreHandshakeClose(unittest.TestCase): |
| 110 | ++ """Verify behavior of close sockets with received data before to the handshake. |
| 111 | ++ """ |
| 112 | ++ |
| 113 | ++ class SingleConnectionTestServerThread(threading.Thread): |
| 114 | ++ |
| 115 | ++ def __init__(self, name=None, call_after_accept=None): |
| 116 | ++ self.call_after_accept = call_after_accept |
| 117 | ++ self.received_data = b'' # set by .run() |
| 118 | ++ self.wrap_error = None # set by .run() |
| 119 | ++ self.listener = None # set by .start() |
| 120 | ++ self.port = None # set by .start() |
| 121 | ++ super().__init__(name=name) |
| 122 | ++ |
| 123 | ++ def __enter__(self): |
| 124 | ++ self.start() |
| 125 | ++ return self |
| 126 | ++ |
| 127 | ++ def __exit__(self, *args): |
| 128 | ++ try: |
| 129 | ++ if self.listener: |
| 130 | ++ self.listener.close() |
| 131 | ++ except OSError: |
| 132 | ++ pass |
| 133 | ++ self.join() |
| 134 | ++ self.wrap_error = None # avoid dangling references |
| 135 | ++ |
| 136 | ++ def start(self): |
| 137 | ++ self.ssl_ctx = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) |
| 138 | ++ self.ssl_ctx.verify_mode = ssl.CERT_REQUIRED |
| 139 | ++ self.ssl_ctx.load_verify_locations(cafile=ONLYCERT) |
| 140 | ++ self.ssl_ctx.load_cert_chain(certfile=ONLYCERT, keyfile=ONLYKEY) |
| 141 | ++ self.listener = socket.socket() |
| 142 | ++ self.port = support.bind_port(self.listener) |
| 143 | ++ self.listener.settimeout(2.0) |
| 144 | ++ self.listener.listen(1) |
| 145 | ++ super().start() |
| 146 | ++ |
| 147 | ++ def run(self): |
| 148 | ++ conn, address = self.listener.accept() |
| 149 | ++ self.listener.close() |
| 150 | ++ with conn: |
| 151 | ++ if self.call_after_accept(conn): |
| 152 | ++ return |
| 153 | ++ try: |
| 154 | ++ tls_socket = self.ssl_ctx.wrap_socket(conn, server_side=True) |
| 155 | ++ except OSError as err: # ssl.SSLError inherits from OSError |
| 156 | ++ self.wrap_error = err |
| 157 | ++ else: |
| 158 | ++ try: |
| 159 | ++ self.received_data = tls_socket.recv(400) |
| 160 | ++ except OSError: |
| 161 | ++ pass # closed, protocol error, etc. |
| 162 | ++ |
| 163 | ++ def non_linux_skip_if_other_okay_error(self, err): |
| 164 | ++ if sys.platform == "linux": |
| 165 | ++ return # Expect the full test setup to always work on Linux. |
| 166 | ++ if (isinstance(err, ConnectionResetError) or |
| 167 | ++ (isinstance(err, OSError) and err.errno == errno.EINVAL) or |
| 168 | ++ re.search('wrong.version.number', getattr(err, "reason", ""), re.I)): |
| 169 | ++ # On Windows the TCP RST leads to a ConnectionResetError |
| 170 | ++ # (ECONNRESET) which Linux doesn't appear to surface to userspace. |
| 171 | ++ # If wrap_socket() winds up on the "if connected:" path and doing |
| 172 | ++ # the actual wrapping... we get an SSLError from OpenSSL. Typically |
| 173 | ++ # WRONG_VERSION_NUMBER. While appropriate, neither is the scenario |
| 174 | ++ # we're specifically trying to test. The way this test is written |
| 175 | ++ # is known to work on Linux. We'll skip it anywhere else that it |
| 176 | ++ # does not present as doing so. |
| 177 | ++ self.skipTest("Could not recreate conditions on %s: %s" % (sys.platform, err)) |
| 178 | ++ # If maintaining this conditional winds up being a problem. |
| 179 | ++ # just turn this into an unconditional skip anything but Linux. |
| 180 | ++ # The important thing is that our CI has the logic covered. |
| 181 | ++ |
| 182 | ++ def test_preauth_data_to_tls_server(self): |
| 183 | ++ server_accept_called = threading.Event() |
| 184 | ++ ready_for_server_wrap_socket = threading.Event() |
| 185 | ++ |
| 186 | ++ def call_after_accept(unused): |
| 187 | ++ server_accept_called.set() |
| 188 | ++ if not ready_for_server_wrap_socket.wait(2.0): |
| 189 | ++ raise RuntimeError("wrap_socket event never set, test may fail.") |
| 190 | ++ return False # Tell the server thread to continue. |
| 191 | ++ |
| 192 | ++ server = self.SingleConnectionTestServerThread( |
| 193 | ++ call_after_accept=call_after_accept, |
| 194 | ++ name="preauth_data_to_tls_server") |
| 195 | ++ server.__enter__() # starts it |
| 196 | ++ self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it. |
| 197 | ++ |
| 198 | ++ with socket.socket() as client: |
| 199 | ++ client.connect(server.listener.getsockname()) |
| 200 | ++ # This forces an immediate connection close via RST on .close(). |
| 201 | ++ set_socket_so_linger_on_with_zero_timeout(client) |
| 202 | ++ client.setblocking(False) |
| 203 | ++ |
| 204 | ++ server_accept_called.wait() |
| 205 | ++ client.send(b"DELETE /data HTTP/1.0\r\n\r\n") |
| 206 | ++ client.close() # RST |
| 207 | ++ |
| 208 | ++ ready_for_server_wrap_socket.set() |
| 209 | ++ server.join() |
| 210 | ++ wrap_error = server.wrap_error |
| 211 | ++ self.assertEqual(b"", server.received_data) |
| 212 | ++ self.assertIsInstance(wrap_error, OSError) # All platforms. |
| 213 | ++ self.non_linux_skip_if_other_okay_error(wrap_error) |
| 214 | ++ self.assertIsInstance(wrap_error, ssl.SSLError) |
| 215 | ++ self.assertIn("before TLS handshake with data", wrap_error.args[1]) |
| 216 | ++ self.assertIn("before TLS handshake with data", wrap_error.reason) |
| 217 | ++ self.assertNotEqual(0, wrap_error.args[0]) |
| 218 | ++ self.assertIsNone(wrap_error.library, msg="attr must exist") |
| 219 | ++ |
| 220 | ++ def test_preauth_data_to_tls_client(self): |
| 221 | ++ client_can_continue_with_wrap_socket = threading.Event() |
| 222 | ++ |
| 223 | ++ def call_after_accept(conn_to_client): |
| 224 | ++ # This forces an immediate connection close via RST on .close(). |
| 225 | ++ set_socket_so_linger_on_with_zero_timeout(conn_to_client) |
| 226 | ++ conn_to_client.send( |
| 227 | ++ b"HTTP/1.0 307 Temporary Redirect\r\n" |
| 228 | ++ b"Location: https://example.com/someone-elses-server\r\n" |
| 229 | ++ b"\r\n") |
| 230 | ++ conn_to_client.close() # RST |
| 231 | ++ client_can_continue_with_wrap_socket.set() |
| 232 | ++ return True # Tell the server to stop. |
| 233 | ++ |
| 234 | ++ server = self.SingleConnectionTestServerThread( |
| 235 | ++ call_after_accept=call_after_accept, |
| 236 | ++ name="preauth_data_to_tls_client") |
| 237 | ++ server.__enter__() # starts it |
| 238 | ++ self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it. |
| 239 | ++ |
| 240 | ++ # Redundant; call_after_accept sets SO_LINGER on the accepted conn. |
| 241 | ++ set_socket_so_linger_on_with_zero_timeout(server.listener) |
| 242 | ++ |
| 243 | ++ with socket.socket() as client: |
| 244 | ++ client.connect(server.listener.getsockname()) |
| 245 | ++ if not client_can_continue_with_wrap_socket.wait(2.0): |
| 246 | ++ self.fail("test server took too long.") |
| 247 | ++ ssl_ctx = ssl.create_default_context() |
| 248 | ++ try: |
| 249 | ++ tls_client = ssl_ctx.wrap_socket( |
| 250 | ++ client, server_hostname="localhost") |
| 251 | ++ except OSError as err: # SSLError inherits from OSError |
| 252 | ++ wrap_error = err |
| 253 | ++ received_data = b"" |
| 254 | ++ else: |
| 255 | ++ wrap_error = None |
| 256 | ++ received_data = tls_client.recv(400) |
| 257 | ++ tls_client.close() |
| 258 | ++ |
| 259 | ++ server.join() |
| 260 | ++ self.assertEqual(b"", received_data) |
| 261 | ++ self.assertIsInstance(wrap_error, OSError) # All platforms. |
| 262 | ++ self.non_linux_skip_if_other_okay_error(wrap_error) |
| 263 | ++ self.assertIsInstance(wrap_error, ssl.SSLError) |
| 264 | ++ self.assertIn("before TLS handshake with data", wrap_error.args[1]) |
| 265 | ++ self.assertIn("before TLS handshake with data", wrap_error.reason) |
| 266 | ++ self.assertNotEqual(0, wrap_error.args[0]) |
| 267 | ++ self.assertIsNone(wrap_error.library, msg="attr must exist") |
| 268 | ++ |
| 269 | ++ def test_https_client_non_tls_response_ignored(self): |
| 270 | ++ |
| 271 | ++ server_responding = threading.Event() |
| 272 | ++ |
| 273 | ++ class SynchronizedHTTPSConnection(httplib.HTTPSConnection): |
| 274 | ++ def connect(self): |
| 275 | ++ httplib.HTTPConnection.connect(self) |
| 276 | ++ # Wait for our fault injection server to have done its thing. |
| 277 | ++ if not server_responding.wait(1.0) and support.verbose: |
| 278 | ++ sys.stdout.write("server_responding event never set.") |
| 279 | ++ self.sock = self._context.wrap_socket( |
| 280 | ++ self.sock, server_hostname=self.host) |
| 281 | ++ |
| 282 | ++ def call_after_accept(conn_to_client): |
| 283 | ++ # This forces an immediate connection close via RST on .close(). |
| 284 | ++ set_socket_so_linger_on_with_zero_timeout(conn_to_client) |
| 285 | ++ conn_to_client.send( |
| 286 | ++ b"HTTP/1.0 402 Payment Required\r\n" |
| 287 | ++ b"\r\n") |
| 288 | ++ conn_to_client.close() # RST |
| 289 | ++ server_responding.set() |
| 290 | ++ return True # Tell the server to stop. |
| 291 | ++ |
| 292 | ++ server = self.SingleConnectionTestServerThread( |
| 293 | ++ call_after_accept=call_after_accept, |
| 294 | ++ name="non_tls_http_RST_responder") |
| 295 | ++ server.__enter__() # starts it |
| 296 | ++ self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it. |
| 297 | ++ # Redundant; call_after_accept sets SO_LINGER on the accepted conn. |
| 298 | ++ set_socket_so_linger_on_with_zero_timeout(server.listener) |
| 299 | ++ |
| 300 | ++ connection = SynchronizedHTTPSConnection( |
| 301 | ++ "localhost", |
| 302 | ++ port=server.port, |
| 303 | ++ context=ssl.create_default_context(), |
| 304 | ++ timeout=2.0, |
| 305 | ++ ) |
| 306 | ++ # There are lots of reasons this raises as desired, long before this |
| 307 | ++ # test was added. Sending the request requires a successful TLS wrapped |
| 308 | ++ # socket; that fails if the connection is broken. It may seem pointless |
| 309 | ++ # to test this. It serves as an illustration of something that we never |
| 310 | ++ # want to happen... properly not happening. |
| 311 | ++ with self.assertRaises(OSError) as err_ctx: |
| 312 | ++ connection.request("HEAD", "/test", headers={"Host": "localhost"}) |
| 313 | ++ response = connection.getresponse() |
| 314 | ++ |
| 315 | ++ |
| 316 | + def test_main(verbose=False): |
| 317 | + if support.verbose: |
| 318 | + plats = { |
| 319 | +Index: Python-2.7.18/Misc/NEWS.d/next/Security/2023-08-22-17-39-12.gh-issue-108310.fVM3sg.rst |
| 320 | +=================================================================== |
| 321 | +--- /dev/null |
| 322 | ++++ Python-2.7.18/Misc/NEWS.d/next/Security/2023-08-22-17-39-12.gh-issue-108310.fVM3sg.rst |
| 323 | +@@ -0,0 +1,7 @@ |
| 324 | ++Fixed an issue where instances of :class:`ssl.SSLSocket` were vulnerable to |
| 325 | ++a bypass of the TLS handshake and included protections (like certificate |
| 326 | ++verification) and treating sent unencrypted data as if it were |
| 327 | ++post-handshake TLS encrypted data. Security issue reported as |
| 328 | ++`CVE-2023-40217 |
| 329 | ++<https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-40217>`_ by |
| 330 | ++Aapo Oksman. Patch by Gregory P. Smith. |
0 commit comments