From 4da070daef1c23be8c3be6e2fb5921b17a23c79a Mon Sep 17 00:00:00 2001 From: Jericho Tolentino <68654047+jericht@users.noreply.github.com> Date: Thu, 29 Feb 2024 13:22:54 -0600 Subject: [PATCH] feat: add macOS socket support and enable macOS CI (#65) --------- Signed-off-by: Jericho Tolentino <68654047+jericht@users.noreply.github.com> Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com> Co-authored-by: Morgan Epp <60796713+epmog@users.noreply.github.com> --- .github/workflows/reuse_python_build.yml | 6 +-- .../adaptor_runtime/_http/request_handler.py | 45 ++++++++++++++++--- src/openjd/adaptor_runtime/_http/sockets.py | 21 +++++++++ src/openjd/adaptor_runtime/_osname.py | 3 +- .../adaptor_runtime_client/connection.py | 44 ++++++++++++++++-- .../unit/http/test_request_handler.py | 8 ++-- .../adaptor_runtime/unit/http/test_sockets.py | 45 +++++++++++++++++++ .../unit/utils/test_secure_open.py | 4 +- .../unit/test_client_interface.py | 2 +- 9 files changed, 159 insertions(+), 19 deletions(-) diff --git a/.github/workflows/reuse_python_build.yml b/.github/workflows/reuse_python_build.yml index 9a345d0..a1e0dc1 100644 --- a/.github/workflows/reuse_python_build.yml +++ b/.github/workflows/reuse_python_build.yml @@ -19,7 +19,7 @@ jobs: strategy: matrix: python-version: ['3.9', '3.10', '3.11'] - os: ["ubuntu-latest", "windows-latest"] + os: ["ubuntu-latest", "windows-latest", "macos-latest"] env: PYTHON: ${{ matrix.python-version }} CODEARTIFACT_REGION: "us-west-2" @@ -48,8 +48,8 @@ jobs: aws-region: us-west-2 mask-aws-account-id: true - - name: Setup CodeArtifact Linux - if: ${{ matrix.os == 'ubuntu-latest'}} + - name: Setup CodeArtifact Unix + if: ${{ matrix.os == 'ubuntu-latest' || matrix.os == 'macos-latest' }} run: | CODEARTIFACT_AUTH_TOKEN=$(aws codeartifact get-authorization-token --domain ${{ secrets.CODEARTIFACT_DOMAIN }} --domain-owner ${{ secrets.CODEARTIFACT_ACCOUNT_ID }} --query authorizationToken --output text --region us-west-2) echo "::add-mask::$CODEARTIFACT_AUTH_TOKEN" diff --git a/src/openjd/adaptor_runtime/_http/request_handler.py b/src/openjd/adaptor_runtime/_http/request_handler.py index 457dc0b..cc8c6b5 100644 --- a/src/openjd/adaptor_runtime/_http/request_handler.py +++ b/src/openjd/adaptor_runtime/_http/request_handler.py @@ -11,8 +11,9 @@ import urllib.parse as urllib_parse from dataclasses import dataclass from http import HTTPStatus, server -from typing import Callable, Type +from typing import Any, Callable, Type +from .._osname import OSName from .exceptions import UnsupportedPlatformException _logger = logging.getLogger(__name__) @@ -120,13 +121,27 @@ def _authenticate(self) -> bool: "Failed to handle request because it was not made through a UNIX socket" ) + peercred_opt_level: Any + peercred_opt: Any + cred_cls: Any + if OSName.is_macos(): # pragma: no cover + # SOL_LOCAL is not defined in Python's socket module, need to hardcode it + # source: https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/un.h#L85 + peercred_opt_level = 0 # type: ignore[attr-defined] + peercred_opt = socket.LOCAL_PEERCRED # type: ignore[attr-defined] + cred_cls = XUCred + else: # pragma: no cover + peercred_opt_level = socket.SOL_SOCKET # type: ignore[attr-defined] + peercred_opt = socket.SO_PEERCRED # type: ignore[attr-defined] + cred_cls = UCred + # Get the credentials of the peer process cred_buffer = self.connection.getsockopt( - socket.SOL_SOCKET, # type: ignore[attr-defined] - socket.SO_PEERCRED, # type: ignore[attr-defined] - socket.CMSG_SPACE(ctypes.sizeof(UCred)), # type: ignore[attr-defined] + peercred_opt_level, + peercred_opt, + socket.CMSG_SPACE(ctypes.sizeof(cred_cls)), # type: ignore[attr-defined] ) - peer_cred = UCred.from_buffer_copy(cred_buffer) + peer_cred = cred_cls.from_buffer_copy(cred_buffer) # Only allow connections from a process running as the same user return peer_cred.uid == os.getuid() # type: ignore[attr-defined] @@ -149,6 +164,26 @@ def __str__(self): # pragma: no cover return f"pid:{self.pid} uid:{self.uid} gid:{self.gid}" +class XUCred(ctypes.Structure): + """ + Represents the xucred struct returned from the LOCAL_PEERCRED socket option. + + For more info, see LOCAL_PEERCRED in the unix(4) man page + """ + + _fields_ = [ + ("version", ctypes.c_uint), + ("uid", ctypes.c_uint), + ("ngroups", ctypes.c_short), + # cr_groups is a uint array of NGROUPS elements, which is defined as 16 + # source: + # - https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/ucred.h#L207 + # - https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/param.h#L100 + # - https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/syslimits.h#L100 + ("groups", ctypes.c_uint * 16), + ] + + @dataclass class HTTPResponse: """ diff --git a/src/openjd/adaptor_runtime/_http/sockets.py b/src/openjd/adaptor_runtime/_http/sockets.py index 4929a35..58e3e34 100644 --- a/src/openjd/adaptor_runtime/_http/sockets.py +++ b/src/openjd/adaptor_runtime/_http/sockets.py @@ -152,8 +152,29 @@ def verify_socket_path(self, path: str) -> None: ) +class MacOSSocketDirectories(SocketDirectories): + """ + Specialization for socket paths in macOS systems. + """ + + # This is based on the max length of socket names to 104 bytes + # See https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/un.h#L79 + _socket_path_max_length = 104 + _socket_dir_max_length = _socket_path_max_length - _PID_MAX_LENGTH_PADDED + + def verify_socket_path(self, path: str) -> None: + path_length = len(path.encode("utf-8")) + if path_length > self._socket_dir_max_length: + raise NonvalidSocketPathException( + "Socket base directory path too big. The maximum allowed size is " + f"{self._socket_dir_max_length} bytes, but the directory has a size of " + f"{path_length}: {path}" + ) + + _os_map: dict[str, type[SocketDirectories]] = { OSName.LINUX: LinuxSocketDirectories, + OSName.MACOS: MacOSSocketDirectories, } diff --git a/src/openjd/adaptor_runtime/_osname.py b/src/openjd/adaptor_runtime/_osname.py index a7ba749..cbe4524 100644 --- a/src/openjd/adaptor_runtime/_osname.py +++ b/src/openjd/adaptor_runtime/_osname.py @@ -37,7 +37,8 @@ def __new__(cls, *args, **kw): return str.__new__(cls, *args, **kw) @staticmethod - def is_macos(name: str) -> bool: + def is_macos(name: Optional[str] = None) -> bool: + name = OSName._get_os_name() if name is None else name return OSName.resolve_os_name(name) == OSName.MACOS @staticmethod diff --git a/src/openjd/adaptor_runtime_client/connection.py b/src/openjd/adaptor_runtime_client/connection.py index c30d681..ff65224 100644 --- a/src/openjd/adaptor_runtime_client/connection.py +++ b/src/openjd/adaptor_runtime_client/connection.py @@ -5,6 +5,8 @@ import socket as _socket import ctypes as _ctypes import os as _os +from sys import platform +from typing import Any from http.client import HTTPConnection as _HTTPConnection @@ -29,6 +31,26 @@ def __str__(self): # pragma: no cover return f"pid:{self.pid} uid:{self.uid} gid:{self.gid}" +class XUCred(_ctypes.Structure): + """ + Represents the xucred struct returned from the LOCAL_PEERCRED socket option. + + For more info, see LOCAL_PEERCRED in the unix(4) man page + """ + + _fields_ = [ + ("version", _ctypes.c_uint), + ("uid", _ctypes.c_uint), + ("ngroups", _ctypes.c_short), + # cr_groups is a uint array of NGROUPS elements, which is defined as 16 + # source: + # - https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/ucred.h#L207 + # - https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/param.h#L100 + # - https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/syslimits.h#L100 + ("groups", _ctypes.c_uint * 16), + ] + + class UnixHTTPConnection(_HTTPConnection): # pragma: no cover """ Specialization of http.client.HTTPConnection class that uses a UNIX domain socket. @@ -61,13 +83,27 @@ def _authenticate(self) -> bool: "Failed to handle request because it was not made through a UNIX socket" ) + peercred_opt_level: Any + peercred_opt: Any + cred_cls: Any + if platform == "darwin": + # SOL_LOCAL is not defined in Python's socket module, need to hardcode it + # source: https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/un.h#L85 + peercred_opt_level = 0 # type: ignore[attr-defined] + peercred_opt = _socket.LOCAL_PEERCRED # type: ignore[attr-defined] + cred_cls = XUCred + else: + peercred_opt_level = _socket.SOL_SOCKET # type: ignore[attr-defined] + peercred_opt = _socket.SO_PEERCRED # type: ignore[attr-defined] + cred_cls = UCred + # Get the credentials of the peer process cred_buffer = self.sock.getsockopt( - _socket.SOL_SOCKET, # type: ignore[attr-defined] - _socket.SO_PEERCRED, # type: ignore[attr-defined] - _socket.CMSG_SPACE(_ctypes.sizeof(UCred)), # type: ignore[attr-defined] + peercred_opt_level, + peercred_opt, + _socket.CMSG_SPACE(_ctypes.sizeof(cred_cls)), # type: ignore[attr-defined] ) - peer_cred = UCred.from_buffer_copy(cred_buffer) + peer_cred = cred_cls.from_buffer_copy(cred_buffer) # Only allow connections from a process running as the same user return peer_cred.uid == _os.getuid() # type: ignore[attr-defined] diff --git a/test/openjd/adaptor_runtime/unit/http/test_request_handler.py b/test/openjd/adaptor_runtime/unit/http/test_request_handler.py index fc9fedb..ac0a117 100644 --- a/test/openjd/adaptor_runtime/unit/http/test_request_handler.py +++ b/test/openjd/adaptor_runtime/unit/http/test_request_handler.py @@ -137,7 +137,7 @@ def test_respond_with_body( mock_wfile.write.assert_called_once_with(body.encode("utf-8")) -@pytest.mark.skipif(not OSName.is_linux(), reason="Linux-specific tests") +@pytest.mark.skipif(not OSName.is_posix(), reason="Posix-specific tests") class TestAuthentication: """ Tests for the RequestHandler authentication @@ -148,6 +148,8 @@ class TestAuthenticate: Tests for the RequestHandler._authenticate() method """ + cred_cls = request_handler.XUCred if OSName.is_macos() else request_handler.UCred + @pytest.fixture def mock_handler(self) -> MagicMock: mock_socket = MagicMock(spec=socket.socket) @@ -159,7 +161,7 @@ def mock_handler(self) -> MagicMock: return mock_handler @patch.object(request_handler.os, "getuid") - @patch.object(request_handler.UCred, "from_buffer_copy") + @patch.object(cred_cls, "from_buffer_copy") def test_accepts_same_uid( self, mock_from_buffer_copy: MagicMock, mock_getuid: MagicMock, mock_handler: MagicMock ) -> None: @@ -174,7 +176,7 @@ def test_accepts_same_uid( assert result @patch.object(request_handler.os, "getuid") - @patch.object(request_handler.UCred, "from_buffer_copy") + @patch.object(cred_cls, "from_buffer_copy") def test_rejects_different_uid( self, mock_from_buffer_copy: MagicMock, mock_getuid: MagicMock, mock_handler: MagicMock ) -> None: diff --git a/test/openjd/adaptor_runtime/unit/http/test_sockets.py b/test/openjd/adaptor_runtime/unit/http/test_sockets.py index 68a218e..f6e9c5f 100644 --- a/test/openjd/adaptor_runtime/unit/http/test_sockets.py +++ b/test/openjd/adaptor_runtime/unit/http/test_sockets.py @@ -11,6 +11,7 @@ import openjd.adaptor_runtime._http.sockets as sockets from openjd.adaptor_runtime._http.sockets import ( LinuxSocketDirectories, + MacOSSocketDirectories, NonvalidSocketPathException, NoSocketPathFoundException, SocketDirectories, @@ -263,3 +264,47 @@ def test_rejects_paths_over_100_bytes(self): f"{subject._socket_dir_max_length} bytes, but the directory has a size of " f"{length}: {path}" ) + + +class TestMacOSSocketDirectories: + @pytest.mark.parametrize( + argnames=["path"], + argvalues=[ + ["a"], + ["a" * 96], + ], + ids=["one byte", "96 bytes"], + ) + def test_accepts_paths_within_100_bytes(self, path: str): + """ + Verifies the function accepts paths up to 96 bytes (104 byte max - 8 byte padding + for socket name portion (path sep + PID)) + """ + # GIVEN + subject = MacOSSocketDirectories() + + try: + # WHEN + subject.verify_socket_path(path) + except NonvalidSocketPathException as e: + pytest.fail(f"verify_socket_path raised an error when it should not have: {e}") + else: + # THEN + pass # success + + def test_rejects_paths_over_96_bytes(self): + # GIVEN + length = 97 + path = "a" * length + subject = MacOSSocketDirectories() + + # WHEN + with pytest.raises(NonvalidSocketPathException) as raised_exc: + subject.verify_socket_path(path) + + # THEN + assert raised_exc.match( + "Socket base directory path too big. The maximum allowed size is " + f"{subject._socket_dir_max_length} bytes, but the directory has a size of " + f"{length}: {path}" + ) diff --git a/test/openjd/adaptor_runtime/unit/utils/test_secure_open.py b/test/openjd/adaptor_runtime/unit/utils/test_secure_open.py index 2d48279..7e45384 100644 --- a/test/openjd/adaptor_runtime/unit/utils/test_secure_open.py +++ b/test/openjd/adaptor_runtime/unit/utils/test_secure_open.py @@ -44,8 +44,8 @@ ], ) @patch.object(os, "open") -@pytest.mark.skipif(not OSName.is_linux(), reason="Linux-specific tests") -def test_secure_open_in_linux(mock_os_open, path, open_mode, mask, expected_os_open_kwargs): +@pytest.mark.skipif(not OSName.is_posix(), reason="Posix-specific tests") +def test_secure_open_in_posix(mock_os_open, path, open_mode, mask, expected_os_open_kwargs): # WHEN with patch("builtins.open", mock_open()) as mocked_open: secure_open_kwargs = {"mask": mask} if mask else {} diff --git a/test/openjd/adaptor_runtime_client/unit/test_client_interface.py b/test/openjd/adaptor_runtime_client/unit/test_client_interface.py index b9e315c..98b66a4 100644 --- a/test/openjd/adaptor_runtime_client/unit/test_client_interface.py +++ b/test/openjd/adaptor_runtime_client/unit/test_client_interface.py @@ -51,7 +51,7 @@ def close(self, args: _Optional[_Dict[str, _Any]]) -> None: @pytest.mark.skipif(not OSName.is_posix(), reason="Posix-specific tests") -class TestLinuxClientInterface: +class TestPosixClientInterface: @pytest.mark.parametrize( argnames=("original_path", "new_path"), argvalues=[