From e713b06c2290e1c4caf9feefa3a161cce3adfd2f Mon Sep 17 00:00:00 2001 From: RobLe3 Date: Sun, 6 Sep 2026 18:04:56 +0200 Subject: [PATCH 1/2] Enforce Windows ACL parity for the opt-in trust store --- CHANGELOG.md | 8 ++ WINDOWS_TRUST_STORE.md | 23 ++++++ src/iicp_client/dispatch_ticket_trust.py | 60 ++++++++++++--- src/iicp_client/windows_private_path.py | 93 +++++++++++++++++++++++ tests/test_dispatch_ticket_trust_store.py | 6 +- tests/test_windows_private_path.py | 71 +++++++++++++++++ 6 files changed, 249 insertions(+), 12 deletions(-) create mode 100644 WINDOWS_TRUST_STORE.md create mode 100644 src/iicp_client/windows_private_path.py create mode 100644 tests/test_windows_private_path.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 467c61a..812041b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ in the main repo). ## [Unreleased] +### Security — Windows dispatch trust-store permissions + +- Verify real Windows owner/LocalSystem DACLs for the opt-in durable trust store. + Create restricted descriptors before writing, refuse broad access and reparse + paths, and fail closed when the bounded security adapter is unavailable. + Ordinary v1 dispatch and POSIX behavior are unchanged. See `WINDOWS_TRUST_STORE.md` + for the durability and local process-overhead boundaries. + ### Fixed — Windows artifact validation - Keep proxy startup compatible with redirected cp1252 consoles. diff --git a/WINDOWS_TRUST_STORE.md b/WINDOWS_TRUST_STORE.md new file mode 100644 index 0000000..f88e24b --- /dev/null +++ b/WINDOWS_TRUST_STORE.md @@ -0,0 +1,23 @@ +# Windows dispatch trust-store boundary + +The opt-in dispatch-ticket v2 file store uses a protected Windows DACL for the +current identity and LocalSystem. Its directory, lock and temporary state files +receive restrictive descriptors at creation. Existing ACLs are verified rather +than silently repaired. Relative Windows paths are made absolute without resolving +away symlinks or junctions. + +Load and update reject broad ACLs, unexpected owners, reparse paths and unavailable +verification tools. The bounded Windows PowerShell adapter follows the same ACL +rules as the maintained TypeScript and Rust stores. It uses a fixed operation and +an encoded path, without an interactive profile or shell interpolation. This does +not prevent administrators from exercising Windows ownership/recovery privileges. + +File data is flushed before atomic replacement. Unix additionally flushes the +parent directory; Windows does not claim that additional crash-durability barrier. +Ordinary recovery tests are not power-loss tests. + +ACL verification starts local processes and has measurable overhead. Applications +can set `lock_timeout_s` explicitly when contention warrants a larger bounded +budget. The serialization fixture uses a fifteen-second Windows contention budget; +the separate zero-timeout fixture still verifies refusal. The default remains two +seconds. No high-throughput or qualification claim follows from positive store tests. diff --git a/src/iicp_client/dispatch_ticket_trust.py b/src/iicp_client/dispatch_ticket_trust.py index e13fa9b..e3eaf36 100644 --- a/src/iicp_client/dispatch_ticket_trust.py +++ b/src/iicp_client/dispatch_ticket_trust.py @@ -16,7 +16,8 @@ from dataclasses import dataclass, field from pathlib import Path from tempfile import NamedTemporaryFile -from typing import Any +from typing import IO, Any +from uuid import uuid4 from cryptography.exceptions import InvalidSignature from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PublicKey @@ -141,16 +142,40 @@ class AdminRecoveryAuthorization: minimum_high_water: int = 0 +def _windows_private_path(path: Path, operation: str) -> None: + from .windows_private_path import windows_private_path + try: + windows_private_path(path, operation) + except PermissionError as exc: + raise TrustBundleStoreError("Windows trust store path is unsafe") from exc + + +def _flush_trust_payload(stream: IO[bytes], payload: bytes) -> None: + stream.write(payload) + stream.flush() + os.fsync(stream.fileno()) + + class FileTrustBundleStore: """Owner-local atomic trust bundle store; never enabled by default dispatch.""" def __init__(self, path: str | Path, *, lock_timeout_s: float = 2.0) -> None: self.path = Path(path).expanduser() + if os.name == "nt": + self.path = self.path.absolute() self.lock_path = self.path.with_name(self.path.name + ".lock") self.lock_timeout_s = max(0.0, lock_timeout_s) def _prepare_directory(self) -> None: - self.path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + if os.name == "nt": + from .windows_private_path import windows_private_path + operation = "directory-check" if self.path.parent.exists() else "directory-create" + try: + windows_private_path(self.path.parent, operation) + except PermissionError as exc: + raise TrustBundleStoreError("Windows trust store directory is unsafe") from exc + else: + self.path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) if self.path.parent.is_symlink() or not self.path.parent.is_dir(): raise TrustBundleStoreError("trust store directory must be a directory, not a link") if _POSIX_MODE_SEMANTICS: @@ -163,7 +188,11 @@ def _acquire_lock(self) -> int: deadline = time.monotonic() + self.lock_timeout_s while True: try: - fd = os.open(self.lock_path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + if os.name == "nt": + _windows_private_path(self.lock_path, "file-create") + fd = os.open(self.lock_path, os.O_WRONLY) + else: + fd = os.open(self.lock_path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) os.write(fd, f"{os.getpid()}\n".encode()) os.fsync(fd) return fd @@ -182,6 +211,13 @@ def _release_lock(self, fd: int) -> None: def load(self) -> StoredTrustBundle | None: if not self.path.exists(): return None + if os.name == "nt": + from .windows_private_path import windows_private_path + try: + windows_private_path(self.path.parent, "directory-check") + windows_private_path(self.path, "file-check") + except PermissionError as exc: + raise TrustBundleStoreCorrupt("Windows trust store path is unsafe") from exc metadata = self.path.lstat() if stat.S_ISLNK(metadata.st_mode) or not stat.S_ISREG(metadata.st_mode): raise TrustBundleStoreCorrupt("trust store must be a regular file, not a link") @@ -235,13 +271,17 @@ def _commit(self, bundle: TrustBundle, high_water: int) -> StoredTrustBundle: "high_water": high_water, } payload = json.dumps(state, sort_keys=True, separators=(",", ":")).encode() - with NamedTemporaryFile(dir=self.path.parent, prefix=self.path.name + ".tmp-", delete=False) as tmp: - tmp_path = Path(tmp.name) - if _POSIX_MODE_SEMANTICS: - os.fchmod(tmp.fileno(), 0o600) - tmp.write(payload) - tmp.flush() - os.fsync(tmp.fileno()) + if os.name == "nt": + tmp_path = self.path.with_name(self.path.name + ".tmp-" + uuid4().hex) + _windows_private_path(tmp_path, "file-create") + with tmp_path.open("wb") as windows_tmp: + _flush_trust_payload(windows_tmp, payload) + else: + with NamedTemporaryFile(dir=self.path.parent, prefix=self.path.name + ".tmp-", delete=False) as tmp: + tmp_path = Path(tmp.name) + if _POSIX_MODE_SEMANTICS: + os.fchmod(tmp.fileno(), 0o600) + _flush_trust_payload(tmp.file, payload) try: os.replace(tmp_path, self.path) if _POSIX_MODE_SEMANTICS: diff --git a/src/iicp_client/windows_private_path.py b/src/iicp_client/windows_private_path.py new file mode 100644 index 0000000..754ca97 --- /dev/null +++ b/src/iicp_client/windows_private_path.py @@ -0,0 +1,93 @@ +"""Windows owner/LocalSystem ACL boundary for the opt-in durable trust store.""" +from __future__ import annotations + +import base64 +import os +import subprocess +from pathlib import Path + +_SCRIPT = r''' +# SPDX-License-Identifier: Apache-2.0 +# Same ACL rules as the maintained TypeScript trust-store boundary. +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version Latest +try { + $path = [Text.Encoding]::UTF8.GetString([Convert]::FromBase64String('__PATH_BASE64__')) + $operation = '__OPERATION__' + $sid = [Security.Principal.WindowsIdentity]::GetCurrent().User + $system = [Security.Principal.SecurityIdentifier]::new('S-1-5-18') + $isDirectory = $operation.StartsWith('directory-') + # Reject aliases before creation as well as before validation. + $cursor = $path + while ($cursor) { + if ([IO.File]::Exists($cursor) -or [IO.Directory]::Exists($cursor)) { + if (([IO.File]::GetAttributes($cursor) -band [IO.FileAttributes]::ReparsePoint) -ne 0) { throw 'REPARSE_PATH' } + } + $parent = [IO.Path]::GetDirectoryName($cursor) + if ($parent -eq $cursor) { break } + $cursor = $parent + } + if ($operation.EndsWith('-create')) { + $security = if ($isDirectory) { [Security.AccessControl.DirectorySecurity]::new() } else { [Security.AccessControl.FileSecurity]::new() } + $security.SetOwner($sid) + $security.SetAccessRuleProtection($true, $false) + $inheritance = if ($isDirectory) { [Security.AccessControl.InheritanceFlags]'ContainerInherit,ObjectInherit' } else { [Security.AccessControl.InheritanceFlags]::None } + foreach ($principal in @($sid, $system)) { + $rule = [Security.AccessControl.FileSystemAccessRule]::new($principal, [Security.AccessControl.FileSystemRights]::FullControl, $inheritance, [Security.AccessControl.PropagationFlags]::None, [Security.AccessControl.AccessControlType]::Allow) + $security.AddAccessRule($rule) + } + if ($isDirectory) { + # The descriptor is applied at creation; existing directories are not repaired. + [IO.Directory]::CreateDirectory($path, $security) | Out-Null + } else { + $stream = [IO.FileStream]::new($path, [IO.FileMode]::CreateNew, [Security.AccessControl.FileSystemRights]::FullControl, [IO.FileShare]::None, 4096, [IO.FileOptions]::None, $security) + $stream.Dispose() + } + } + $attributes = [IO.File]::GetAttributes($path) + if (($attributes -band [IO.FileAttributes]::ReparsePoint) -ne 0) { throw 'REPARSE_PATH' } + if ((($attributes -band [IO.FileAttributes]::Directory) -ne 0) -ne $isDirectory) { throw 'PATH_KIND' } + $acl = if ($isDirectory) { [IO.Directory]::GetAccessControl($path) } else { [IO.File]::GetAccessControl($path) } + if ($acl.GetOwner([Security.Principal.SecurityIdentifier]).Value -ne $sid.Value) { throw 'OWNER_DIFFERS' } + $ownerFull = $false + foreach ($rule in $acl.GetAccessRules($true, $true, [Security.Principal.SecurityIdentifier])) { + if ($rule.AccessControlType -ne [Security.AccessControl.AccessControlType]::Allow) { throw 'DENY_RULE' } + if ($rule.IdentityReference.Value -notin @($sid.Value, $system.Value)) { throw 'BROAD_ACCESS' } + if ($rule.IdentityReference.Value -eq $sid.Value -and -not ($rule.PropagationFlags -band [Security.AccessControl.PropagationFlags]::InheritOnly) -and ($rule.FileSystemRights -band [Security.AccessControl.FileSystemRights]::FullControl) -eq [Security.AccessControl.FileSystemRights]::FullControl) { $ownerFull = $true } + } + if (-not $ownerFull) { throw 'OWNER_ACCESS_MISSING' } + [Console]::Out.Write('OK') +} catch { + $exception = $_.Exception + while ($exception.InnerException) { $exception = $exception.InnerException } + $errorCode = $exception.HResult -band 0xffff + if ($operation -eq 'file-create' -and $errorCode -in @(80, 183)) { [Console]::Out.Write('EXISTS'); exit 0 } + [Console]::Out.Write('REFUSED'); exit 1 +} +''' + + +def windows_private_path(path: Path, operation: str) -> None: + if not path.is_absolute() or "\0" in str(path): + raise PermissionError("Windows private path must be absolute") + if operation not in {"directory-create", "directory-check", "file-create", "file-check"}: + raise PermissionError("Invalid Windows private-path operation") + root = Path(os.environ.get("SystemRoot", "")) + if not root.is_absolute(): + raise PermissionError("Windows security tool unavailable") + script = _SCRIPT.replace("__PATH_BASE64__", base64.b64encode(str(path).encode()).decode()) + script = script.replace("__OPERATION__", operation) + try: + result = subprocess.run( + [str(root / "System32/WindowsPowerShell/v1.0/powershell.exe"), + "-NoLogo", "-NoProfile", "-NonInteractive", "-EncodedCommand", + base64.b64encode(script.encode("utf-16le")).decode()], + stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, + timeout=10, check=True, + ).stdout + except (OSError, subprocess.SubprocessError) as exc: + raise PermissionError("Windows private-path verification failed") from exc + if result.strip() == b"EXISTS": + raise FileExistsError("Windows private file already exists") + if result.strip() != b"OK": + raise PermissionError("Windows private-path verification failed") diff --git a/tests/test_dispatch_ticket_trust_store.py b/tests/test_dispatch_ticket_trust_store.py index 1a874bc..4c05a99 100644 --- a/tests/test_dispatch_ticket_trust_store.py +++ b/tests/test_dispatch_ticket_trust_store.py @@ -116,7 +116,9 @@ def test_concurrent_writers_never_finish_below_highest_version(tmp_path: Path) - def install(bundle: TrustBundle) -> None: barrier.wait() - statuses.append(FileTrustBundleStore(path).install(bundle).status) + # ACL subprocess overhead is measured separately from serialization. + timeout = 15.0 if os.name == "nt" else 2.0 + statuses.append(FileTrustBundleStore(path, lock_timeout_s=timeout).install(bundle).status) threads = [threading.Thread(target=install, args=(bundle,)) for bundle in (v2, v3)] for thread in threads: @@ -134,7 +136,7 @@ def install(bundle: TrustBundle) -> None: def test_held_lock_times_out_without_mutating_state(tmp_path: Path) -> None: path = tmp_path / "trust" / "bundle.state" store = FileTrustBundleStore(path, lock_timeout_s=0) - path.parent.mkdir(mode=0o700) + store._prepare_directory() store.lock_path.write_text("held", encoding="utf-8") os.chmod(store.lock_path, 0o600) diff --git a/tests/test_windows_private_path.py b/tests/test_windows_private_path.py new file mode 100644 index 0000000..ba1564c --- /dev/null +++ b/tests/test_windows_private_path.py @@ -0,0 +1,71 @@ +from __future__ import annotations + +import base64 +import os +import subprocess +from pathlib import Path +from unittest.mock import Mock + +import pytest + +from iicp_client.dispatch_ticket_trust import FileTrustBundleStore, TrustBundle, TrustBundleStoreError +from iicp_client.windows_private_path import windows_private_path + + +def test_encoded_input_and_tool_failure_are_bounded(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("SystemRoot", str(tmp_path)) + run = Mock(return_value=subprocess.CompletedProcess([], 0, b"OK")) + monkeypatch.setattr(subprocess, "run", run) + path = tmp_path / "quoted'$;path" + windows_private_path(path, "file-check") + args, kwargs = run.call_args + script = base64.b64decode(args[0][-1]).decode("utf-16le") + assert str(path) not in script + assert kwargs["timeout"] == 10 + with pytest.raises(PermissionError): + windows_private_path(path, "bad'") + with pytest.raises(PermissionError): + windows_private_path(Path("relative"), "file-check") + run.side_effect = subprocess.TimeoutExpired("security-tool", 10) + with pytest.raises(PermissionError): + windows_private_path(path, "file-check") + run.side_effect = FileNotFoundError() + with pytest.raises(PermissionError): + windows_private_path(path, "file-check") + run.side_effect = None + run.return_value = subprocess.CompletedProcess([], 0, b"EXISTS") + with pytest.raises(FileExistsError): + windows_private_path(path, "file-create") + + +@pytest.mark.skipif(os.name != "nt", reason="Windows DACL/reparse semantics") +def test_windows_store_rejects_broad_acl_and_junction(tmp_path: Path) -> None: + path = tmp_path / "private" / "bundle.state" + store = FileTrustBundleStore(path) + bundle = TrustBundle.from_dict({"bundle_version": 1, "keys": []}) + store.install(bundle) + before = path.read_bytes() + tool = str(Path(os.environ["SystemRoot"]) / "System32/icacls.exe") + subprocess.run([tool, str(path), "/grant", "*S-1-1-0:R"], check=True, capture_output=True, timeout=10) + with pytest.raises(TrustBundleStoreError): + store.load() + assert path.read_bytes() == before + subprocess.run([tool, str(path), "/remove:g", "*S-1-1-0"], check=True, capture_output=True, timeout=10) + assert store.load() is not None + alias = tmp_path / "alias" + encode = lambda p: base64.b64encode(str(p).encode()).decode() # noqa: E731 + script = ( + "$ErrorActionPreference='Stop'; " + f"$a=[Text.Encoding]::UTF8.GetString([Convert]::FromBase64String('{encode(alias)}')); " + f"$p=[Text.Encoding]::UTF8.GetString([Convert]::FromBase64String('{encode(path.parent)}')); " + "New-Item -ItemType Junction -Path $a -Target $p | Out-Null" + ) + powershell = str(Path(os.environ["SystemRoot"]) / "System32/WindowsPowerShell/v1.0/powershell.exe") + subprocess.run([powershell, "-NoProfile", "-NonInteractive", "-EncodedCommand", + base64.b64encode(script.encode("utf-16le")).decode()], + check=True, capture_output=True, timeout=10) + try: + with pytest.raises(TrustBundleStoreError): + FileTrustBundleStore(alias / "bundle.state").load() + finally: + alias.rmdir() From ee540acee090f6cd548b25363a4e05581d4f90fc Mon Sep 17 00:00:00 2001 From: RobLe3 Date: Sun, 6 Sep 2026 18:06:06 +0200 Subject: [PATCH 2/2] Require both trust-store writer threads to complete --- tests/test_dispatch_ticket_trust_store.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_dispatch_ticket_trust_store.py b/tests/test_dispatch_ticket_trust_store.py index 4c05a99..b86fcc9 100644 --- a/tests/test_dispatch_ticket_trust_store.py +++ b/tests/test_dispatch_ticket_trust_store.py @@ -130,6 +130,7 @@ def install(bundle: TrustBundle) -> None: state = store.load() assert state is not None assert state.bundle.bundle_version == state.high_water == 3 + assert len(statuses) == 2, "both concurrent writers must finish without hidden thread failures" assert set(statuses) <= {"installed", "stale"}