From e27977b95c28e7e0706037140e5f484bfb978858 Mon Sep 17 00:00:00 2001 From: Jericho Tolentino <68654047+jericht@users.noreply.github.com> Date: Tue, 30 Jan 2024 04:08:03 +0000 Subject: [PATCH 1/2] feat: add macOS socket support and enable macOS CI Signed-off-by: Jericho Tolentino <68654047+jericht@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 +++++++++++++++++++ 7 files changed, 156 insertions(+), 16 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..cfc7f30 100644 --- a/src/openjd/adaptor_runtime_client/connection.py +++ b/src/openjd/adaptor_runtime_client/connection.py @@ -5,7 +5,9 @@ import socket as _socket import ctypes as _ctypes import os as _os +from typing import Any from http.client import HTTPConnection as _HTTPConnection +from ..adaptor_runtime._osname import OSName class UnrecognizedBackgroundConnectionError(Exception): @@ -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 OSName.is_macos(): + # 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..3d469b8 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_linux() or not OSName.is_macos(), reason="Unix-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}" + ) From cb145d2f8b789dc30dc7198c7400b2c0c37f9a80 Mon Sep 17 00:00:00 2001 From: Morgan Epp <60796713+epmog@users.noreply.github.com> Date: Wed, 28 Feb 2024 19:31:01 -0600 Subject: [PATCH 2/2] fix: remove relative import since client can't handle it Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com> --- src/openjd/adaptor_runtime_client/connection.py | 4 ++-- test/openjd/adaptor_runtime/unit/http/test_request_handler.py | 2 +- test/openjd/adaptor_runtime/unit/utils/test_secure_open.py | 4 ++-- .../adaptor_runtime_client/unit/test_client_interface.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/openjd/adaptor_runtime_client/connection.py b/src/openjd/adaptor_runtime_client/connection.py index cfc7f30..ff65224 100644 --- a/src/openjd/adaptor_runtime_client/connection.py +++ b/src/openjd/adaptor_runtime_client/connection.py @@ -5,9 +5,9 @@ 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 -from ..adaptor_runtime._osname import OSName class UnrecognizedBackgroundConnectionError(Exception): @@ -86,7 +86,7 @@ def _authenticate(self) -> bool: peercred_opt_level: Any peercred_opt: Any cred_cls: Any - if OSName.is_macos(): + 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] 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 3d469b8..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() or not OSName.is_macos(), reason="Unix-specific tests") +@pytest.mark.skipif(not OSName.is_posix(), reason="Posix-specific tests") class TestAuthentication: """ Tests for the RequestHandler authentication 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=[