Bound generated UDS socket paths
Actor names and custom runtime subdirs can otherwise produce unsafe or overlong pathname sockets after moving Darwin's bindspace to `/tmp`. Deats, - hash unsafe or over-budget actor names while retaining `@pid.sock` - share deterministic naming with post-kill socket cleanup - enforce Linux and Darwin `sun_path` byte budgets - restore non-Darwin dir validation and mode `0700` - validate every nested Darwin runtime-dir component Review: PR #480 (goodboy) https://github.com/goodboy/tractor/pull/480 (this patch was generated in some part by `opencode` using `gpt-5.6-sol` (`openai`))wkt/uds_macos_473
parent
64e820e18e
commit
340d506940
|
|
@ -97,6 +97,143 @@ def test_macos_rt_dir_rejects_symlink(
|
|||
assert stat.S_IMODE(target_dir.stat().st_mode) == 0o755
|
||||
|
||||
|
||||
def test_rt_dir_rejects_non_directory(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
):
|
||||
'''
|
||||
Preserve the non-Darwin runtime-directory type contract.
|
||||
|
||||
Replacing `Path.is_dir()` with unguarded `lstat()` briefly made
|
||||
existing files look like valid runtime directories on Linux.
|
||||
This test points `platformdirs` at a regular file and proves
|
||||
`get_rt_dir()` rejects it during initialization.
|
||||
|
||||
'''
|
||||
rt_file: Path = tmp_path / 'runtime-file'
|
||||
rt_file.touch()
|
||||
monkeypatch.setattr(sys, 'platform', 'linux')
|
||||
monkeypatch.setattr(
|
||||
'platformdirs.user_runtime_dir',
|
||||
lambda appname: str(rt_file),
|
||||
)
|
||||
|
||||
with pytest.raises(FileExistsError):
|
||||
_state.get_rt_dir()
|
||||
|
||||
new_rt_dir: Path = tmp_path / 'new-runtime-dir'
|
||||
monkeypatch.setattr(
|
||||
'platformdirs.user_runtime_dir',
|
||||
lambda appname: str(new_rt_dir),
|
||||
)
|
||||
assert _state.get_rt_dir() == new_rt_dir
|
||||
assert stat.S_IMODE(new_rt_dir.stat().st_mode) == 0o700
|
||||
|
||||
|
||||
def test_macos_rt_dir_rejects_intermediate_symlink(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
):
|
||||
'''
|
||||
Reject symlinks in nested Darwin runtime subdirectories.
|
||||
|
||||
The earlier final-component check allowed `link/child` to follow
|
||||
an intermediate symlink and create `child` outside the secured
|
||||
runtime root. This test installs that link and proves traversal
|
||||
stops before anything is created in its target.
|
||||
|
||||
'''
|
||||
rt_root: Path = tmp_path / f'tractor-{os.getuid()}'
|
||||
target_dir: Path = tmp_path / 'target'
|
||||
rt_root.mkdir(mode=0o700)
|
||||
target_dir.mkdir()
|
||||
(rt_root / 'link').symlink_to(
|
||||
target_dir,
|
||||
target_is_directory=True,
|
||||
)
|
||||
monkeypatch.setattr(sys, 'platform', 'darwin')
|
||||
monkeypatch.setattr(_state, '_DARWIN_TMPDIR', tmp_path)
|
||||
|
||||
with pytest.raises(PermissionError, match='Unsafe Darwin'):
|
||||
_state.get_rt_dir(subdir='link/child')
|
||||
|
||||
assert not (target_dir / 'child').exists()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
('platform_name', 'path_limit'),
|
||||
[
|
||||
('darwin', 104),
|
||||
('linux', 108),
|
||||
],
|
||||
)
|
||||
def test_uds_sockname_compaction(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
platform_name: str,
|
||||
path_limit: int,
|
||||
):
|
||||
'''
|
||||
Keep generated actor sockets safe and below Darwin's byte limit.
|
||||
|
||||
Actor names are unrestricted identity strings. A long, multibyte,
|
||||
or path-like name previously produced overlong or escaping socket
|
||||
paths. These cases prove `UDSAddress.get_sockname()` preserves a
|
||||
short legacy name, deterministically compacts unsafe names, keeps
|
||||
the reaper's `@pid.sock` suffix, and stays within Darwin's byte
|
||||
limit.
|
||||
|
||||
'''
|
||||
from tractor.ipc._uds import UDSAddress
|
||||
|
||||
bindspace: Path = Path('/tmp/tractor-501')
|
||||
pid: int = 12345
|
||||
from tractor.ipc import _uds
|
||||
|
||||
monkeypatch.setattr(sys, 'platform', platform_name)
|
||||
monkeypatch.setattr(_uds, '_SUN_PATH_LIMIT', path_limit)
|
||||
|
||||
short: Path = UDSAddress.get_sockname(
|
||||
name='worker',
|
||||
pid=pid,
|
||||
bindspace=bindspace,
|
||||
)
|
||||
long_name: str = 'actor-' + ('\u00e9' * 100)
|
||||
compact: Path = UDSAddress.get_sockname(
|
||||
name=long_name,
|
||||
pid=pid,
|
||||
bindspace=bindspace,
|
||||
)
|
||||
unsafe: Path = UDSAddress.get_sockname(
|
||||
name='../worker',
|
||||
pid=pid,
|
||||
bindspace=bindspace,
|
||||
)
|
||||
|
||||
assert short == Path(f'worker@{pid}.sock')
|
||||
assert compact == UDSAddress.get_sockname(
|
||||
name=long_name,
|
||||
pid=pid,
|
||||
bindspace=bindspace,
|
||||
)
|
||||
assert compact.name.endswith(f'@{pid}.sock')
|
||||
assert unsafe.parent == Path('.')
|
||||
assert '..' not in unsafe.name
|
||||
assert len(os.fsencode(bindspace / compact)) < path_limit
|
||||
|
||||
with pytest.raises(ValueError) as exc_info:
|
||||
UDSAddress.get_sockname(
|
||||
name=long_name,
|
||||
pid=pid,
|
||||
bindspace=Path('/tmp') / ('x' * 90),
|
||||
)
|
||||
|
||||
errmsg: str = str(exc_info.value)
|
||||
assert 'leaves no room' in errmsg
|
||||
assert 'name was unsafe: False' in errmsg
|
||||
assert 'name was over budget: True' in errmsg
|
||||
assert f'AF_UNIX path limit: {path_limit}' in errmsg
|
||||
|
||||
|
||||
def test_uds_bindspace_created_implicitly(
|
||||
debug_mode: bool,
|
||||
bindspace_dir_str: str,
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ from __future__ import annotations
|
|||
from contextlib import (
|
||||
contextmanager as cm,
|
||||
)
|
||||
import hashlib
|
||||
from pathlib import Path
|
||||
import os
|
||||
import sys
|
||||
|
|
@ -96,6 +97,12 @@ else:
|
|||
|
||||
log = get_logger()
|
||||
|
||||
_SUN_PATH_LIMIT: int = (
|
||||
108
|
||||
if sys.platform == 'linux'
|
||||
else 104
|
||||
)
|
||||
|
||||
|
||||
def unwrap_sockpath(
|
||||
sockpath: Path,
|
||||
|
|
@ -196,7 +203,7 @@ class UDSAddress(
|
|||
err_on_no_runtime=False,
|
||||
)
|
||||
if actor:
|
||||
sockname: str = f'{actor.aid.name}@{pid}'
|
||||
sockname: str = actor.aid.name
|
||||
# XXX, orig version which broke both macOS (file-name
|
||||
# length) and `multiaddrs` ('::' invalid separator).
|
||||
# sockname: str = '::'.join(actor.uid) + f'@{pid}'
|
||||
|
|
@ -222,15 +229,72 @@ class UDSAddress(
|
|||
# `(?P<name>.+)@(?P<pid>\d+)\.sock` regex, and the
|
||||
# `spawn._reap` `{name}@{pid}.sock` reconstruction.
|
||||
token: str = uuid4().hex[:8]
|
||||
sockname: str = f'{prefix}.{token}@{pid}'
|
||||
sockname = f'{prefix}.{token}'
|
||||
|
||||
sockpath: Path = Path(f'{sockname}.sock')
|
||||
sockpath: Path = cls.get_sockname(
|
||||
name=sockname,
|
||||
pid=pid,
|
||||
bindspace=filedir,
|
||||
)
|
||||
return UDSAddress(
|
||||
filedir=filedir,
|
||||
filename=sockpath,
|
||||
maybe_pid=pid,
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def get_sockname(
|
||||
cls,
|
||||
name: str,
|
||||
pid: int,
|
||||
bindspace: Path,
|
||||
) -> Path:
|
||||
'''
|
||||
Build a safe, deterministic UDS socket filename.
|
||||
|
||||
'''
|
||||
suffix: str = f'@{pid}.sock'
|
||||
filename: str = f'{name}{suffix}'
|
||||
unsafe: bool = (
|
||||
'\0' in name
|
||||
or
|
||||
'/' in name
|
||||
or
|
||||
bool(os.altsep and os.altsep in name)
|
||||
or
|
||||
Path(filename).is_absolute()
|
||||
)
|
||||
too_long: bool = (
|
||||
len(os.fsencode(bindspace / filename))
|
||||
>= _SUN_PATH_LIMIT
|
||||
)
|
||||
if (
|
||||
unsafe
|
||||
or
|
||||
too_long
|
||||
):
|
||||
digest: str = hashlib.blake2s(
|
||||
os.fsencode(name),
|
||||
digest_size=16,
|
||||
).hexdigest()
|
||||
filename = f'actor.{digest}{suffix}'
|
||||
|
||||
sockpath: Path = bindspace / filename
|
||||
path_nbytes: int = len(os.fsencode(sockpath))
|
||||
if path_nbytes >= _SUN_PATH_LIMIT:
|
||||
raise ValueError(
|
||||
f'UDS bindspace leaves no room for an AF_UNIX '
|
||||
f'socket filename!\n'
|
||||
f'bindspace: {bindspace}\n'
|
||||
f'name was unsafe: {unsafe}\n'
|
||||
f'name was over budget: {too_long}\n'
|
||||
f'compacted filename: {filename}\n'
|
||||
f'encoded path bytes: {path_nbytes}\n'
|
||||
f'AF_UNIX path limit: {_SUN_PATH_LIMIT}\n'
|
||||
)
|
||||
|
||||
return Path(filename)
|
||||
|
||||
@classmethod
|
||||
def get_root(cls) -> UDSAddress:
|
||||
def_uds_filename: Path = 'registry@1616.sock'
|
||||
|
|
|
|||
|
|
@ -383,6 +383,7 @@ def get_rt_dir(
|
|||
# Normalize and validate that `subdir` is a relative path
|
||||
# without any parent-directory ("..") components, to prevent
|
||||
# escaping the runtime directory.
|
||||
subdir_path: Path|None = None
|
||||
if subdir:
|
||||
subdir_path = (
|
||||
subdir
|
||||
|
|
@ -400,32 +401,46 @@ def get_rt_dir(
|
|||
f'{subdir!r}\n'
|
||||
)
|
||||
|
||||
rt_dir: Path = rt_dir / subdir_path
|
||||
|
||||
try:
|
||||
dir_stat: os.stat_result = rt_dir.lstat()
|
||||
except FileNotFoundError:
|
||||
rt_dir.mkdir(
|
||||
mode=0o700,
|
||||
parents=True,
|
||||
exist_ok=True, # avoid `FileExistsError` from conc calls
|
||||
)
|
||||
dir_stat = rt_dir.lstat()
|
||||
|
||||
if rt_root is not None:
|
||||
if (
|
||||
not stat.S_ISDIR(dir_stat.st_mode)
|
||||
or
|
||||
dir_stat.st_uid != os.getuid()
|
||||
):
|
||||
raise PermissionError(
|
||||
f'Unsafe Darwin runtime directory!\n'
|
||||
f'path: {rt_dir}\n'
|
||||
f'owner uid: {dir_stat.st_uid}\n'
|
||||
f'mode: {stat.filemode(dir_stat.st_mode)}\n'
|
||||
if rt_root is None:
|
||||
if subdir_path is not None:
|
||||
rt_dir = rt_dir / subdir_path
|
||||
if not rt_dir.is_dir():
|
||||
rt_dir.mkdir(
|
||||
# Runtime dirs hold IPC sockets; owner-only access
|
||||
# prevents other users from traversing the bindspace.
|
||||
mode=0o700,
|
||||
parents=True,
|
||||
exist_ok=True,
|
||||
)
|
||||
if rt_dir != rt_root:
|
||||
rt_dir.relative_to(rt_root)
|
||||
return rt_dir
|
||||
|
||||
if subdir_path is not None:
|
||||
for part in subdir_path.parts:
|
||||
rt_dir = rt_dir / part
|
||||
try:
|
||||
dir_stat: os.stat_result = rt_dir.lstat()
|
||||
except FileNotFoundError:
|
||||
try:
|
||||
# Every Darwin component is private so no other
|
||||
# user can replace descendants below `rt_root`.
|
||||
rt_dir.mkdir(mode=0o700)
|
||||
except FileExistsError:
|
||||
pass
|
||||
dir_stat = rt_dir.lstat()
|
||||
|
||||
if (
|
||||
not stat.S_ISDIR(dir_stat.st_mode)
|
||||
or
|
||||
dir_stat.st_uid != os.getuid()
|
||||
):
|
||||
raise PermissionError(
|
||||
f'Unsafe Darwin runtime directory!\n'
|
||||
f'path: {rt_dir}\n'
|
||||
f'owner uid: {dir_stat.st_uid}\n'
|
||||
f'mode: {stat.filemode(dir_stat.st_mode)}\n'
|
||||
)
|
||||
if stat.S_IMODE(dir_stat.st_mode) != 0o700:
|
||||
rt_dir.chmod(0o700)
|
||||
|
||||
return rt_dir
|
||||
|
||||
|
|
|
|||
|
|
@ -71,6 +71,7 @@ fd leak. Different bug class but same broader theme of
|
|||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import trio
|
||||
|
|
@ -154,7 +155,11 @@ def unlink_uds_bind_addrs(
|
|||
and subactor is not None
|
||||
and proc.pid is not None
|
||||
):
|
||||
sockname: str = f'{subactor.aid.name}@{proc.pid}.sock'
|
||||
sockname: Path = UDSAddress.get_sockname(
|
||||
name=subactor.aid.name,
|
||||
pid=proc.pid,
|
||||
bindspace=UDSAddress.def_bindspace,
|
||||
)
|
||||
sockpath: str = str(
|
||||
UDSAddress.def_bindspace / sockname
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in New Issue