Authenticate socket connection for socket.socketpair() fallback

Co-authored-by: Gregory P. Smith <[email protected]>

Author: sethmlarson

Lib/socket.py

@@ -52,7 +52,7 @@
 import _socket
 from _socket import *
 
-import os, sys, io, selectors
+import os, sys, io, selectors, time
 from enum import IntEnum, IntFlag
 
 try:
@@ -628,28 +628,81 @@ def socketpair(family=AF_INET, type=SOCK_STREAM, proto=0):
         if proto != 0:
             raise ValueError("Only protocol zero is supported")
 
-        # We create a connected TCP socket. Note the trick with
-        # setblocking(False) that prevents us from having to create a thread.
-        lsock = socket(family, type, proto)
-        try:
-            lsock.bind((host, 0))
-            lsock.listen()
-            # On IPv6, ignore flow_info and scope_id
-            addr, port = lsock.getsockname()[:2]
-            csock = socket(family, type, proto)
+        import secrets  # Delay import until actually needed.
+        auth_len = secrets.DEFAULT_ENTROPY
+        reason = "unknown"
+        for _ in range(5):
+            # We do not try forever, that'd just provide another process
+            # the ability to make us waste CPU retrying.  In all normal
+            # circumstances the first connection auth attempt succeeds.
+            s_auth = secrets.token_bytes()
+            c_auth = secrets.token_bytes()
+
+            # We create a connected TCP socket. Note the trick with
+            # setblocking(False) prevents us from having to create a thread.
+            csock = None
+            ssock = None
+            reason = "unknown"
+            lsock = socket(family, type, proto)
             try:
-                csock.setblocking(False)
+                lsock.bind((host, 0))
+                lsock.listen()
+                # On IPv6, ignore flow_info and scope_id
+                addr, port = lsock.getsockname()[:2]
+                csock = socket(family, type, proto)
                 try:
-                    csock.connect((addr, port))
-                except (BlockingIOError, InterruptedError):
-                    pass
-                csock.setblocking(True)
-                ssock, _ = lsock.accept()
-            except:
-                csock.close()
-                raise
-        finally:
-            lsock.close()
+                    csock.setblocking(False)
+                    try:
+                        csock.connect((addr, port))
+                    except (BlockingIOError, InterruptedError):
+                        pass
+                    csock.setblocking(True)
+                    ssock, _ = lsock.accept()
+                except Exception:
+                    csock.close()
+                    raise
+
+                def authenticate_socket_conn(send_sock, recv_sock, auth_bytes):
+                    nonlocal auth_len
+                    data_buf = bytearray(auth_len)
+                    data_mem = memoryview(data_buf)
+                    data_len = 0
+
+                    # Send the authentication bytes.
+                    if send_sock.send(auth_bytes) != auth_len:
+                        raise ConnectionError("send() sent too few auth bytes.")
+
+                    # Attempt to read the authentication bytes from the socket.
+                    max_auth_time = time.monotonic() + 3.0
+                    while time.monotonic() < max_auth_time and data_len < auth_len:
+                        bytes_received = recv_sock.recv_into(data_mem, auth_len - data_len)
+                        if bytes_received == 0:
+                            break  # Connection closed.
+                        data_len += bytes_received
+                        data_mem = data_mem[bytes_received:]
+
+                    # Check that the authentication bytes match.
+                    if len(data_buf) != auth_len:
+                        raise ConnectionError("recv() got too few auth bytes.")
+                    if bytes(data_buf) != auth_bytes:
+                        raise ConnectionError(f"Mismatched auth token.")
+
+                # Authenticating avoids using a connection from something else
+                # able to connect to {host}:{port} instead of us.
+                try:
+                    authenticate_socket_conn(ssock, csock, s_auth)
+                    authenticate_socket_conn(csock, ssock, c_auth)
+                except OSError as exc:
+                    csock.close()
+                    ssock.close()
+                    reason = str(exc)
+                    continue
+                # authentication successful, both sockets are our process.
+                break
+            finally:
+                lsock.close()
+        else:
+            raise ConnectionError(f"socketpair authentication failed: {reason}")
         return (ssock, csock)
     __all__.append("socketpair")
 

Lib/test/test_socket.py

@@ -592,19 +592,27 @@ class SocketPairTest(unittest.TestCase, ThreadableTest):
     def __init__(self, methodName='runTest'):
         unittest.TestCase.__init__(self, methodName=methodName)
         ThreadableTest.__init__(self)
+        self.cli = None
+        self.serv = None
+
+    def call_socketpair(self):
+        # To be overridden by some child classes.
+        return socket.socketpair()
 
     def setUp(self):
-        self.serv, self.cli = socket.socketpair()
+        self.serv, self.cli = self.call_socketpair()
 
     def tearDown(self):
-        self.serv.close()
+        if self.serv:
+            self.serv.close()
         self.serv = None
 
     def clientSetUp(self):
         pass
 
     def clientTearDown(self):
-        self.cli.close()
+        if self.cli:
+            self.cli.close()
         self.cli = None
         ThreadableTest.clientTearDown(self)
 
@@ -4852,6 +4860,84 @@ def _testSend(self):
         self.assertEqual(msg, MSG)
 
 
+class PurePythonSocketPairTest(SocketPairTest):
+
+    # Explicitly use socketpair AF_INET or AF_INET6 to to ensure that is the
+    # code path we're using regardless platform is the pure python one where
+    # `_socket.socketpair` does not exist.  (AF_INET does not work with
+    # _socket.socketpair on many platforms).
+    def call_socketpair(self):
+        # called by super().setUp().
+        try:
+            return socket.socketpair(socket.AF_INET6)
+        except OSError:
+            return socket.socketpair(socket.AF_INET)
+
+    # Local imports in this class make for easy security fix backporting.
+
+    def setUp(self):
+        import _socket
+        self._orig_sp = getattr(_socket, 'socketpair', None)
+        if self._orig_sp is not None:
+            # This forces the version using the non-OS provided socketpair
+            # emulation via an AF_INET socket in Lib/socket.py.
+            del _socket.socketpair
+            import importlib
+            global socket
+            socket = importlib.reload(socket)
+        else:
+            pass  # This platform already uses the non-OS provided version.
+        super().setUp()
+
+    def tearDown(self):
+        super().tearDown()
+        import _socket
+        if self._orig_sp is not None:
+            # Restore the default socket.socketpair definition.
+            _socket.socketpair = self._orig_sp
+            import importlib
+            global socket
+            socket = importlib.reload(socket)
+
+    def test_recv(self):
+        msg = self.serv.recv(1024)
+        self.assertEqual(msg, MSG)
+
+    def _test_recv(self):
+        self.cli.send(MSG)
+
+    def test_send(self):
+        self.serv.send(MSG)
+
+    def _test_send(self):
+        msg = self.cli.recv(1024)
+        self.assertEqual(msg, MSG)
+
+    def _test_injected_authentication_failure(self):
+        # No-op.  Exists for base class threading infrastructure to call.
+        # We could refactor this test into its own lesser class along with the
+        # setUp and tearDown code to construct an ideal; it is simpler to keep
+        # it here and live with extra overhead one this _one_ failure test.
+        pass
+
+    def test_injected_authentication_failure(self):
+        import secrets
+        orig_token_bytes = secrets.token_bytes
+        fake_n = secrets.DEFAULT_ENTROPY - 1
+        from unittest import mock
+        with mock.patch.object(
+                secrets, 'token_bytes',
+                new=lambda nbytes=None: orig_token_bytes(fake_n)):
+            s1, s2 = None, None
+            try:
+                with self.assertRaisesRegex(ConnectionError,
+                                            "authentication fail"):
+                    s1, s2 = socket.socketpair()
+            finally:
+                if s1: s1.close()
+                if s2: s2.close()
+
+
 class NonBlockingTCPTests(ThreadedTCPSocketTest):
 
     def __init__(self, methodName='runTest'):

Misc/NEWS.d/next/Security/2024-07-22-13-11-28.gh-issue-122133.0mPeta.rst

@@ -0,0 +1,5 @@
+Authenticate the socket connection for the ``socket.socketpair()`` fallback
+on platforms where ``AF_UNIX`` is not available like Windows.
+
+Patch by Gregory P. Smith <[email protected]>. Reported by Ellie
+<[email protected]>