| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 38b489b commit 264b1da
3 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1003,7 +1003,7 @@ def _create(cls, sock, server_side=False, do_handshake_on_connect=True, | |||
| 1003 | 1003 | ) | |
| 1004 | 1004 | self = cls.__new__(cls, **kwargs) | |
| 1005 | 1005 | super(SSLSocket, self).__init__(**kwargs) | |
| 1006 | - self.settimeout(sock.gettimeout()) | ||
| 1006 | + sock_timeout = sock.gettimeout() | ||
| 1007 | 1007 | sock.detach() | |
| 1008 | 1008 | ||
| 1009 | 1009 | self._context = context | |
@@ -1022,9 +1022,38 @@ def _create(cls, sock, server_side=False, do_handshake_on_connect=True, | |||
| 1022 | 1022 | if e.errno != errno.ENOTCONN: | |
| 1023 | 1023 | raise | |
| 1024 | 1024 | connected = False | |
| 1025 | + blocking = self.getblocking() | ||
| 1026 | + self.setblocking(False) | ||
| 1027 | + try: | ||
| 1028 | + # We are not connected so this is not supposed to block, but | ||
| 1029 | + # testing revealed otherwise on macOS and Windows so we do | ||
| 1030 | + # the non-blocking dance regardless. Our raise when any data | ||
| 1031 | + # is found means consuming the data is harmless. | ||
| 1032 | + notconn_pre_handshake_data = self.recv(1) | ||
| 1033 | + except OSError as e: | ||
| 1034 | + # EINVAL occurs for recv(1) on non-connected on unix sockets. | ||
| 1035 | + if e.errno not in (errno.ENOTCONN, errno.EINVAL): | ||
| 1036 | + raise | ||
| 1037 | + notconn_pre_handshake_data = b'' | ||
| 1038 | + self.setblocking(blocking) | ||
| 1039 | + if notconn_pre_handshake_data: | ||
| 1040 | + # This prevents pending data sent to the socket before it was | ||
| 1041 | + # closed from escaping to the caller who could otherwise | ||
| 1042 | + # presume it came through a successful TLS connection. | ||
| 1043 | + reason = "Closed before TLS handshake with data in recv buffer." | ||
| 1044 | + notconn_pre_handshake_data_error = SSLError(e.errno, reason) | ||
| 1045 | + # Add the SSLError attributes that _ssl.c always adds. | ||
| 1046 | + notconn_pre_handshake_data_error.reason = reason | ||
| 1047 | + notconn_pre_handshake_data_error.library = None | ||
| 1048 | + try: | ||
| 1049 | + self.close() | ||
| 1050 | + except OSError: | ||
| 1051 | + pass | ||
| 1052 | + raise notconn_pre_handshake_data_error | ||
| 1025 | 1053 | else: | |
| 1026 | 1054 | connected = True | |
| 1027 | 1055 | ||
| 1056 | + self.settimeout(sock_timeout) # Must come after setblocking() calls. | ||
| 1028 | 1057 | self._connected = connected | |
| 1029 | 1058 | if connected: | |
| 1030 | 1059 | # create the SSL object | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -5,11 +5,14 @@ | |||
| 5 | 5 | import unittest.mock | |
| 6 | 6 | from test import support | |
| 7 | 7 | from test.support import socket_helper, warnings_helper | |
| 8 | + import re | ||
| 8 | 9 | import socket | |
| 9 | 10 | import select | |
| 11 | + import struct | ||
| 10 | 12 | import time | |
| 11 | 13 | import datetime | |
| 12 | 14 | import gc | |
| 15 | + import http.client | ||
| 13 | 16 | import os | |
| 14 | 17 | import errno | |
| 15 | 18 | import pprint | |
@@ -4842,6 +4845,218 @@ def sni_cb(sock, servername, ctx): | |||
| 4842 | 4845 | s.connect((HOST, server.port)) | |
| 4843 | 4846 | ||
| 4844 | 4847 | ||
| 4848 | + def set_socket_so_linger_on_with_zero_timeout(sock): | ||
| 4849 | + sock.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack('ii', 1, 0)) | ||
| 4850 | + | ||
| 4851 | + | ||
| 4852 | + class TestPreHandshakeClose(unittest.TestCase): | ||
| 4853 | + """Verify behavior of close sockets with received data before to the handshake. | ||
| 4854 | + """ | ||
| 4855 | + | ||
| 4856 | + class SingleConnectionTestServerThread(threading.Thread): | ||
| 4857 | + | ||
| 4858 | + def __init__(self, *, name, call_after_accept): | ||
| 4859 | + self.call_after_accept = call_after_accept | ||
| 4860 | + self.received_data = b'' # set by .run() | ||
| 4861 | + self.wrap_error = None # set by .run() | ||
| 4862 | + self.listener = None # set by .start() | ||
| 4863 | + self.port = None # set by .start() | ||
| 4864 | + super().__init__(name=name) | ||
| 4865 | + | ||
| 4866 | + def __enter__(self): | ||
| 4867 | + self.start() | ||
| 4868 | + return self | ||
| 4869 | + | ||
| 4870 | + def __exit__(self, *args): | ||
| 4871 | + try: | ||
| 4872 | + if self.listener: | ||
| 4873 | + self.listener.close() | ||
| 4874 | + except OSError: | ||
| 4875 | + pass | ||
| 4876 | + self.join() | ||
| 4877 | + self.wrap_error = None # avoid dangling references | ||
| 4878 | + | ||
| 4879 | + def start(self): | ||
| 4880 | + self.ssl_ctx = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) | ||
| 4881 | + self.ssl_ctx.verify_mode = ssl.CERT_REQUIRED | ||
| 4882 | + self.ssl_ctx.load_verify_locations(cafile=ONLYCERT) | ||
| 4883 | + self.ssl_ctx.load_cert_chain(certfile=ONLYCERT, keyfile=ONLYKEY) | ||
| 4884 | + self.listener = socket.socket() | ||
| 4885 | + self.port = socket_helper.bind_port(self.listener) | ||
| 4886 | + self.listener.settimeout(2.0) | ||
| 4887 | + self.listener.listen(1) | ||
| 4888 | + super().start() | ||
| 4889 | + | ||
| 4890 | + def run(self): | ||
| 4891 | + conn, address = self.listener.accept() | ||
| 4892 | + self.listener.close() | ||
| 4893 | + with conn: | ||
| 4894 | + if self.call_after_accept(conn): | ||
| 4895 | + return | ||
| 4896 | + try: | ||
| 4897 | + tls_socket = self.ssl_ctx.wrap_socket(conn, server_side=True) | ||
| 4898 | + except OSError as err: # ssl.SSLError inherits from OSError | ||
| 4899 | + self.wrap_error = err | ||
| 4900 | + else: | ||
| 4901 | + try: | ||
| 4902 | + self.received_data = tls_socket.recv(400) | ||
| 4903 | + except OSError: | ||
| 4904 | + pass # closed, protocol error, etc. | ||
| 4905 | + | ||
| 4906 | + def non_linux_skip_if_other_okay_error(self, err): | ||
| 4907 | + if sys.platform == "linux": | ||
| 4908 | + return # Expect the full test setup to always work on Linux. | ||
| 4909 | + if (isinstance(err, ConnectionResetError) or | ||
| 4910 | + (isinstance(err, OSError) and err.errno == errno.EINVAL) or | ||
| 4911 | + re.search('wrong.version.number', getattr(err, "reason", ""), re.I)): | ||
| 4912 | + # On Windows the TCP RST leads to a ConnectionResetError | ||
| 4913 | + # (ECONNRESET) which Linux doesn't appear to surface to userspace. | ||
| 4914 | + # If wrap_socket() winds up on the "if connected:" path and doing | ||
| 4915 | + # the actual wrapping... we get an SSLError from OpenSSL. Typically | ||
| 4916 | + # WRONG_VERSION_NUMBER. While appropriate, neither is the scenario | ||
| 4917 | + # we're specifically trying to test. The way this test is written | ||
| 4918 | + # is known to work on Linux. We'll skip it anywhere else that it | ||
| 4919 | + # does not present as doing so. | ||
| 4920 | + self.skipTest(f"Could not recreate conditions on {sys.platform}:" | ||
| 4921 | + f" {err=}") | ||
| 4922 | + # If maintaining this conditional winds up being a problem. | ||
| 4923 | + # just turn this into an unconditional skip anything but Linux. | ||
| 4924 | + # The important thing is that our CI has the logic covered. | ||
| 4925 | + | ||
| 4926 | + def test_preauth_data_to_tls_server(self): | ||
| 4927 | + server_accept_called = threading.Event() | ||
| 4928 | + ready_for_server_wrap_socket = threading.Event() | ||
| 4929 | + | ||
| 4930 | + def call_after_accept(unused): | ||
| 4931 | + server_accept_called.set() | ||
| 4932 | + if not ready_for_server_wrap_socket.wait(2.0): | ||
| 4933 | + raise RuntimeError("wrap_socket event never set, test may fail.") | ||
| 4934 | + return False # Tell the server thread to continue. | ||
| 4935 | + | ||
| 4936 | + server = self.SingleConnectionTestServerThread( | ||
| 4937 | + call_after_accept=call_after_accept, | ||
| 4938 | + name="preauth_data_to_tls_server") | ||
| 4939 | + server.__enter__() # starts it | ||
| 4940 | + self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it. | ||
| 4941 | + | ||
| 4942 | + with socket.socket() as client: | ||
| 4943 | + client.connect(server.listener.getsockname()) | ||
| 4944 | + # This forces an immediate connection close via RST on .close(). | ||
| 4945 | + set_socket_so_linger_on_with_zero_timeout(client) | ||
| 4946 | + client.setblocking(False) | ||
| 4947 | + | ||
| 4948 | + server_accept_called.wait() | ||
| 4949 | + client.send(b"DELETE /data HTTP/1.0\r\n\r\n") | ||
| 4950 | + client.close() # RST | ||
| 4951 | + | ||
| 4952 | + ready_for_server_wrap_socket.set() | ||
| 4953 | + server.join() | ||
| 4954 | + wrap_error = server.wrap_error | ||
| 4955 | + self.assertEqual(b"", server.received_data) | ||
| 4956 | + self.assertIsInstance(wrap_error, OSError) # All platforms. | ||
| 4957 | + self.non_linux_skip_if_other_okay_error(wrap_error) | ||
| 4958 | + self.assertIsInstance(wrap_error, ssl.SSLError) | ||
| 4959 | + self.assertIn("before TLS handshake with data", wrap_error.args[1]) | ||
| 4960 | + self.assertIn("before TLS handshake with data", wrap_error.reason) | ||
| 4961 | + self.assertNotEqual(0, wrap_error.args[0]) | ||
| 4962 | + self.assertIsNone(wrap_error.library, msg="attr must exist") | ||
| 4963 | + | ||
| 4964 | + def test_preauth_data_to_tls_client(self): | ||
| 4965 | + client_can_continue_with_wrap_socket = threading.Event() | ||
| 4966 | + | ||
| 4967 | + def call_after_accept(conn_to_client): | ||
| 4968 | + # This forces an immediate connection close via RST on .close(). | ||
| 4969 | + set_socket_so_linger_on_with_zero_timeout(conn_to_client) | ||
| 4970 | + conn_to_client.send( | ||
| 4971 | + b"HTTP/1.0 307 Temporary Redirect\r\n" | ||
| 4972 | + b"Location: https://example.com/someone-elses-server\r\n" | ||
| 4973 | + b"\r\n") | ||
| 4974 | + conn_to_client.close() # RST | ||
| 4975 | + client_can_continue_with_wrap_socket.set() | ||
| 4976 | + return True # Tell the server to stop. | ||
| 4977 | + | ||
| 4978 | + server = self.SingleConnectionTestServerThread( | ||
| 4979 | + call_after_accept=call_after_accept, | ||
| 4980 | + name="preauth_data_to_tls_client") | ||
| 4981 | + server.__enter__() # starts it | ||
| 4982 | + self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it. | ||
| 4983 | + | ||
| 4984 | + # Redundant; call_after_accept sets SO_LINGER on the accepted conn. | ||
| 4985 | + set_socket_so_linger_on_with_zero_timeout(server.listener) | ||
| 4986 | + | ||
| 4987 | + with socket.socket() as client: | ||
| 4988 | + client.connect(server.listener.getsockname()) | ||
| 4989 | + if not client_can_continue_with_wrap_socket.wait(2.0): | ||
| 4990 | + self.fail("test server took too long.") | ||
| 4991 | + ssl_ctx = ssl.create_default_context() | ||
| 4992 | + try: | ||
| 4993 | + tls_client = ssl_ctx.wrap_socket( | ||
| 4994 | + client, server_hostname="localhost") | ||
| 4995 | + except OSError as err: # SSLError inherits from OSError | ||
| 4996 | + wrap_error = err | ||
| 4997 | + received_data = b"" | ||
| 4998 | + else: | ||
| 4999 | + wrap_error = None | ||
| 5000 | + received_data = tls_client.recv(400) | ||
| 5001 | + tls_client.close() | ||
| 5002 | + | ||
| 5003 | + server.join() | ||
| 5004 | + self.assertEqual(b"", received_data) | ||
| 5005 | + self.assertIsInstance(wrap_error, OSError) # All platforms. | ||
| 5006 | + self.non_linux_skip_if_other_okay_error(wrap_error) | ||
| 5007 | + self.assertIsInstance(wrap_error, ssl.SSLError) | ||
| 5008 | + self.assertIn("before TLS handshake with data", wrap_error.args[1]) | ||
| 5009 | + self.assertIn("before TLS handshake with data", wrap_error.reason) | ||
| 5010 | + self.assertNotEqual(0, wrap_error.args[0]) | ||
| 5011 | + self.assertIsNone(wrap_error.library, msg="attr must exist") | ||
| 5012 | + | ||
| 5013 | + def test_https_client_non_tls_response_ignored(self): | ||
| 5014 | + | ||
| 5015 | + server_responding = threading.Event() | ||
| 5016 | + | ||
| 5017 | + class SynchronizedHTTPSConnection(http.client.HTTPSConnection): | ||
| 5018 | + def connect(self): | ||
| 5019 | + http.client.HTTPConnection.connect(self) | ||
| 5020 | + # Wait for our fault injection server to have done its thing. | ||
| 5021 | + if not server_responding.wait(1.0) and support.verbose: | ||
| 5022 | + sys.stdout.write("server_responding event never set.") | ||
| 5023 | + self.sock = self._context.wrap_socket( | ||
| 5024 | + self.sock, server_hostname=self.host) | ||
| 5025 | + | ||
| 5026 | + def call_after_accept(conn_to_client): | ||
| 5027 | + # This forces an immediate connection close via RST on .close(). | ||
| 5028 | + set_socket_so_linger_on_with_zero_timeout(conn_to_client) | ||
| 5029 | + conn_to_client.send( | ||
| 5030 | + b"HTTP/1.0 402 Payment Required\r\n" | ||
| 5031 | + b"\r\n") | ||
| 5032 | + conn_to_client.close() # RST | ||
| 5033 | + server_responding.set() | ||
| 5034 | + return True # Tell the server to stop. | ||
| 5035 | + | ||
| 5036 | + server = self.SingleConnectionTestServerThread( | ||
| 5037 | + call_after_accept=call_after_accept, | ||
| 5038 | + name="non_tls_http_RST_responder") | ||
| 5039 | + server.__enter__() # starts it | ||
| 5040 | + self.addCleanup(server.__exit__) # ... & unittest.TestCase stops it. | ||
| 5041 | + # Redundant; call_after_accept sets SO_LINGER on the accepted conn. | ||
| 5042 | + set_socket_so_linger_on_with_zero_timeout(server.listener) | ||
| 5043 | + | ||
| 5044 | + connection = SynchronizedHTTPSConnection( | ||
| 5045 | + f"localhost", | ||
| 5046 | + port=server.port, | ||
| 5047 | + context=ssl.create_default_context(), | ||
| 5048 | + timeout=2.0, | ||
| 5049 | + ) | ||
| 5050 | + # There are lots of reasons this raises as desired, long before this | ||
| 5051 | + # test was added. Sending the request requires a successful TLS wrapped | ||
| 5052 | + # socket; that fails if the connection is broken. It may seem pointless | ||
| 5053 | + # to test this. It serves as an illustration of something that we never | ||
| 5054 | + # want to happen... properly not happening. | ||
| 5055 | + with self.assertRaises(OSError) as err_ctx: | ||
| 5056 | + connection.request("HEAD", "/test", headers={"Host": "localhost"}) | ||
| 5057 | + response = connection.getresponse() | ||
| 5058 | + | ||
| 5059 | + | ||
| 4845 | 5060 | def setUpModule(): | |
| 4846 | 5061 | if support.verbose: | |
| 4847 | 5062 | plats = { | |
| Original file line number | Diff line number | Diff 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. | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments