diff --git a/deploy/sps.env.example b/deploy/sps.env.example index ed8b478..5bd8045 100644 --- a/deploy/sps.env.example +++ b/deploy/sps.env.example @@ -5,6 +5,10 @@ SP_HOST = "127.0.0.1" SP_PORT = "8743" +# Non-loopback listeners require an explicit opt-in. +#SP_ALLOW_NONLOOPBACK = "1" +# Maximum UTF-8 bytes in a runtime secret write (positive TOML integer). +#SP_MAX_VALUE_BYTES = 65536 SP_TLS_CERT = "/etc/toolstack/sps.crt" SP_TLS_KEY = "/etc/toolstack/sps.key" SP_TLS_CA = "/etc/toolstack/sps-ca.crt" @@ -40,3 +44,8 @@ VAULT_FILE = "/var/lib/toolstack/sps.vault.json" # ...or: #SP_VAULT_PASSPHRASE_FILE = /etc/toolstack/sps.vault.passphrase # Use an owner-owned mode-0600 file in a mode-0700 parent; canonical path, no links. + +# Optional exact backend items beyond each tool's own id. Keep after all +# top-level settings and outside plugin tables; no wildcard expansion. +#[grants] +#my_tool = ["shared-service", "team/provider-path"] diff --git a/docs/sps.md b/docs/sps.md index 774d372..b11be41 100644 --- a/docs/sps.md +++ b/docs/sps.md @@ -1,7 +1,7 @@ # SPS — Secrets Procurement Service SPS authorizes a registered tool's declared secret names and writable mappings. -The runner registers using SP_SECRET (broad authority); each tool receives an +The runner registers using SP_SECRET, subject to SPS's item grant policy; each tool receives an E_SECRET capability for its registration. Broker channel credentials must be separate from SPS capabilities: runner independently randomizes them and broker reads dedicated channels.json without E_SECRET/private-state fallback. Trusted @@ -16,6 +16,22 @@ cache plaintext. TLS protects transport, not endpoint memory. Registrations are volatile; SPS restart needs runner re-registration and failed-recovery retries. Revocation does not erase already cached values. +By default a registration can bind only its own tool id as the backend `item` +(omitting `item` has the same meaning). To authorize a shared item or provider +path, add an explicit allowlist to the private TOML configuration: + +```toml +[grants] +my_tool = ["shared-service", "team/provider-path"] +``` + +These entries supplement the tool's own item; they are exact strings, not globs. +Unlisted cross-tool items return `Unauthorized`. Existing registrations remain +in memory until replaced, removed or SPS restarts; config is loaded at startup. +Keep the `[grants]` table after top-level settings and outside plugin tables. +SPS still trusts the registrar to choose fields and writable entries within +allowed items. + Supported release: dedicated Toolstack VM containing only Toolstack and trusted tools under its account, agents outside the host reaching only the broker. Same-machine/account agents are local/dev only; this is not hostile-code isolation. @@ -50,6 +66,12 @@ reload; noncooperating writers remain unsafe. Runtime tool writes use SPS writeback and require explicit writable grants. Response redaction is a separate concern; storing a returned token does not automatically hide the response. +Both provisioning commands accept `SP_VAULT_PASSPHRASE_FILE` as well as the direct +environment value (the direct value takes precedence for these commands and the +runtime plugin). New vaults use scrypt N=65536, r=8, p=1 with a 128 MiB memory +ceiling. Existing vaults retain their stored parameters on reads and writes. +`init-vault --force` destroys existing contents; it is not an in-place re-key. + ## Providers and operations Plugins: `localfile`, `infisical`, `hashicorp_vault`; select exactly one at startup. @@ -62,9 +84,27 @@ every malformed/rejected request is recorded. Keep audit values out of logs and protect copies. Rejected credentials, unavailable backend, failed verification or missing registration must be diagnosed without logging secret material. +Successful registrations record hashed field/item pairs, entry count and writable +count. Ping, malformed requests and failed authentication use constant-space, +rate-bounded counters rather than individual audit lines. Summaries flush at most +once per second (up to four counter categories); they are best effort. Audit +rotation retains five generations of up to 4 MiB each plus the current file. + +TLS handshakes and request bodies have separate absolute five-second deadlines. +Writes above 65536 UTF-8 bytes return `Bad request` before reaching the provider; +set top-level `SP_MAX_VALUE_BYTES` to a positive TOML integer to change the limit. +The wire's 1 MiB request limit still applies. Non-loopback `SP_HOST` requires +explicit top-level `SP_ALLOW_NONLOOPBACK = "1"`; loopback remains the default. +New self-signed loopback certificates last ten years; startup warns when fewer +than 30 days remain. Existing certificates are not changed automatically. Use +`regen-tls`, restart SPS, and redistribute the new CA to clients when rotating. +The SDK's fake test path refuses to run whenever `TOOLSTACK_E_SECRET` is present. + On restart, check SPS readiness then runner recovery before invoking tools. A -`Not found` can mean absent registration or missing declared name; it is not proof -the underlying provider value is absent. TLS errors mean check CA/SAN/readability, +`Unauthorized` covers both missing registration and a wrong E_SECRET. `Not found` +means an authenticated caller requested an undeclared name; it is not proof +the underlying provider value is absent. Provider failures return `Backend error`, +including failures during bulk reads. TLS errors mean check CA/SAN/readability, not opt out of verification. Broker schema3 receipts, manual seven-day terminal-payload pruning and private diff --git a/sps/audit.py b/sps/audit.py index 70f9e1d..321a140 100644 --- a/sps/audit.py +++ b/sps/audit.py @@ -1,9 +1,8 @@ """Append-only audit log. -One JSON object per line; the guidance specifies tool_id, secret_name, action, -and timestamp. SP_SECRET and E_SECRET are NEVER logged. New-value fields -for writebacks are filtered out at the caller; we only record (tool_id, -secret_name, action, ts). +One JSON object per line, with bounded rotated history. Credentials and secret +values are never accepted. Registration metadata contains only hashed field/item +identifiers and counts. """ from __future__ import annotations @@ -14,18 +13,15 @@ from pathlib import Path from .paths import private_parent, validate, open_private -# Audit fields we accept. Anything else is silently dropped (defense in depth). -# Notably `esecret`, `spsecret`, `value`, and any other secret-bearing kwarg is -# never persisted -- callers cannot accidentally log a secret because the -# `event()` signature only exposes the safe three. -_ALLOWED = {"tool_id", "secret_name", "action"} - - class AuditLogger: - def __init__(self, path: str | Path, *, max_bytes: int = 4 * 1024 * 1024) -> None: + def __init__(self, path: str | Path, *, max_bytes: int = 4 * 1024 * 1024, + generations: int = 5) -> None: if max_bytes < 1024: raise ValueError("audit max_bytes must be at least 1024") self._max_bytes = max_bytes + if generations < 1: + raise ValueError("audit generations must be positive") + self._generations = generations self._path = Path(path) self._lock = threading.Lock() # If the path looks writable, ensure its parent exists. For tests that @@ -33,12 +29,14 @@ def __init__(self, path: str | Path, *, max_bytes: int = 4 * 1024 * 1024) -> Non # silently no-op. private_parent(self._path) validate(self._path) - validate(str(self._path) + ".1") + for index in range(1, generations + 1): + validate(str(self._path) + f".{index}") def event(self, action: str, *, tool_id: str = "", secret_name: str = "", - outcome: str = "ok", count: int | None = None) -> None: - """Append one event line. Extra kwargs are silently dropped so a future - caller's accidental `esecret=...` cannot leak the value.""" + outcome: str = "ok", count: int | None = None, + entries: list | None = None, entry_count: int | None = None, + writable_count: int | None = None) -> None: + """Append one event line; unknown keyword arguments are refused.""" rec: dict = {"ts": int(time.time()), "action": action[:64], "outcome": outcome[:64]} if tool_id: rec["tool_id"] = tool_id[:128] @@ -46,6 +44,13 @@ def event(self, action: str, *, tool_id: str = "", secret_name: str = "", rec["secret_name"] = secret_name[:128] if count is not None: rec["count"] = max(0, min(int(count), 2**31 - 1)) + if entries is not None: + import re + if any(set(e) != {"field", "item"} or any( + not isinstance(v, str) or not re.fullmatch(r"sha256:[0-9a-f]{64}", v) + for v in e.values()) for e in entries): + raise ValueError("audit entries require hashed identifiers") + rec.update(entries=entries, entry_count=int(entry_count), writable_count=int(writable_count)) payload = (json.dumps(rec) + "\n").encode("utf-8") if len(payload) > self._max_bytes: raise ValueError("audit event exceeds retention limit") @@ -55,7 +60,12 @@ def event(self, action: str, *, tool_id: str = "", secret_name: str = "", try: if os.fstat(fd).st_size + len(payload) > self._max_bytes: validate(self._path) - validate(str(self._path) + ".1") + for index in range(1, self._generations + 1): + validate(str(self._path) + f".{index}") + for index in range(self._generations - 1, 0, -1): + source = str(self._path) + f".{index}" + if os.path.exists(source): + os.replace(source, str(self._path) + f".{index + 1}") os.replace(self._path, str(self._path) + ".1") os.close(fd) fd = -1 diff --git a/sps/cli.py b/sps/cli.py index 2419615..9a9d0f3 100644 --- a/sps/cli.py +++ b/sps/cli.py @@ -81,7 +81,7 @@ def maybe_generate_tls_material(cert_path: str, key_path: str, ca_path: str, sub cmd = [ "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-keyout", key_path, "-out", cert_path, - "-days", "365", "-subj", subj, + "-days", "3650", "-subj", subj, "-addext", "subjectAltName=IP:127.0.0.1,DNS:localhost", ] subprocess.run(cmd, check=True, capture_output=True, umask=0o077) @@ -93,6 +93,14 @@ def maybe_generate_tls_material(cert_path: str, key_path: str, ca_path: str, sub return True +def warn_certificate_expiry(cert_path: str) -> None: + import ssl + import time + cert = ssl._ssl._test_decode_cert(cert_path) + if ssl.cert_time_to_seconds(cert["notAfter"]) - time.time() < 30 * 86400: + print("sps: TLS certificate expires in fewer than 30 days; renew and restart clients", file=sys.stderr) + + def _cmd_serve(args) -> None: """Run the SPS server. Mode-0600 re-check fails closed.""" cfg_path = args.config @@ -102,6 +110,7 @@ def _cmd_serve(args) -> None: raise ValueError("SPS requires TLS certificate and key") ssl_ctx = _ssl.SSLContext(_ssl.PROTOCOL_TLS_SERVER) ssl_ctx.load_cert_chain(certfile=cfg.sp_tls_cert, keyfile=cfg.sp_tls_key) + warn_certificate_expiry(cfg.sp_tls_cert) plugin = plugin_loader.load_plugin(cfg) plugin.connect() audit_log_path = cfg.sp_audit_log or "/var/log/toolstack/sps.audit" @@ -118,7 +127,7 @@ def _cmd_serve(args) -> None: # Announce a fresh boot_id so the toolyard watchdog (admin side) can detect # this restart and re-register every running tool. Without this, SPS's # in-memory tool pool starts empty on every startup and any tool that was - # already running gets "Not found" until it's manually bounced. + # already running gets "Unauthorized" until it is registered again. boot_id_path = Path( os.environ.get("TOOLSTACK_SPS_BOOT_ID_PATH") or _DEFAULT_BOOT_ID_PATH ) @@ -269,10 +278,8 @@ def _cmd_vault_set(args) -> None: value = getpass.getpass(f"value for {args.tool}.{args.field}: ") else: value = sys.stdin.readline().rstrip("\n") - passphrase = os.environ.get("SP_VAULT_PASSPHRASE") - if not passphrase: - raise SystemExit("SP_VAULT_PASSPHRASE not set in env") - from .plugins.localfile import _Vault + from .plugins.localfile import _Vault, _vault_passphrase + passphrase = _vault_passphrase() _Vault(cfg.localfile.vault_file, passphrase).set(args.tool, args.field, value) print(f"set {args.tool}.{args.field}") @@ -282,10 +289,8 @@ def _cmd_vault_get(args) -> None: cfg = cfgmod.load_config(args.config) if cfg.sp_plugin != "localfile" or cfg.localfile is None: raise SystemExit("sps vault-get requires SP_PLUGIN=localfile") - passphrase = os.environ.get("SP_VAULT_PASSPHRASE") - if not passphrase: - raise SystemExit("SP_VAULT_PASSPHRASE not set in env") - from .plugins.localfile import _Vault + from .plugins.localfile import _Vault, _vault_passphrase + passphrase = _vault_passphrase() backend = _Vault(cfg.localfile.vault_file, passphrase) print("set" if backend.has(args.tool, args.field) else "unset") diff --git a/sps/client.py b/sps/client.py index 0e65630..319daae 100644 --- a/sps/client.py +++ b/sps/client.py @@ -61,6 +61,9 @@ def _default_context(**kwargs): class SPSClient: + def __repr__(self) -> str: + return "SPSClient(credentials=)" + def __init__(self, host: str, port: int, *, sp_secret: str | None = None, esecret: str | None = None, ca_file: str | None = None, verify: bool = True, timeout: float = 15.0, diff --git a/sps/config.py b/sps/config.py index e0dbbf0..0eabb1e 100644 --- a/sps/config.py +++ b/sps/config.py @@ -10,7 +10,7 @@ import os import stat import tomllib -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path from urllib.parse import urlsplit import ipaddress @@ -62,13 +62,13 @@ class InfisicalBlock: environment: str = "prod" organization_slug: str | None = None client_id: str = "" - client_secret: str = "" + client_secret: str = field(default="", repr=False) @dataclass(frozen=True) class VaultBlock: url: str - token: str + token: str = field(repr=False) mount: str = "secret" @@ -81,7 +81,7 @@ class LocalFileBlock: class Config: sp_host: str sp_port: int - sp_secret: str + sp_secret: str = field(repr=False) sp_tls_cert: str sp_tls_key: str sp_tls_ca: str @@ -90,6 +90,19 @@ class Config: infisical: InfisicalBlock | None vault: VaultBlock | None localfile: LocalFileBlock | None + grants: dict[str, tuple[str, ...]] = field(default_factory=dict) + sp_max_value_bytes: int = 65536 + sp_allow_nonloopback: bool = False + + +def validate_listener(host: str, allow_nonloopback: bool = False) -> None: + local = host == "localhost" + try: + local = local or ipaddress.ip_address(host).is_loopback + except ValueError: + pass + if not local and not allow_nonloopback: + raise ValueError("non-loopback SP_HOST requires SP_ALLOW_NONLOOPBACK") _ALLOWED_PLUGINS = ("infisical", "hashicorp_vault", "localfile") @@ -121,6 +134,17 @@ def _need(key: str) -> str: return v sp_host = _need("SP_HOST") + allow_nonloopback = strict_bool(data.get("SP_ALLOW_NONLOOPBACK", False)) + validate_listener(sp_host, allow_nonloopback) + grants = data.get("grants", {}) + if not isinstance(grants, dict) or any( + not isinstance(items, list) or any(not isinstance(item, str) or not item for item in items) + for items in grants.values() + ): + raise ValueError("grants must map tool ids to arrays of non-empty items") + max_value = data.get("SP_MAX_VALUE_BYTES", 65536) + if isinstance(max_value, bool) or not isinstance(max_value, int) or max_value < 1: + raise ValueError("SP_MAX_VALUE_BYTES must be a positive integer") sp_port = int(_need("SP_PORT")) sp_secret = _need("SP_SECRET") sp_tls_cert = data.get("SP_TLS_CERT", "") @@ -178,4 +202,7 @@ def _need(key: str) -> str: infisical=infisical, vault=vault, localfile=localfile, + grants={tool: tuple(items) for tool, items in grants.items()}, + sp_max_value_bytes=max_value, + sp_allow_nonloopback=allow_nonloopback, ) diff --git a/sps/handlers.py b/sps/handlers.py index 661fc3d..8402c49 100644 --- a/sps/handlers.py +++ b/sps/handlers.py @@ -91,15 +91,13 @@ def check_sp(ctx: HandlerContext, msg: dict[str, Any]) -> None: def check_es(ctx: HandlerContext, tool_id: str, msg: dict[str, Any]) -> Registration: - """Verify the E_SECRET matches the registered tool. Raises AuthError / - ToolNotFound on any failure.""" + """Verify E_SECRET without disclosing whether a tool is registered.""" val = msg.get(_ES_PREFIX) if not isinstance(val, str) or not val: raise AuthError(f"{_ES_PREFIX} missing or not a string") rec = ctx.store.get(tool_id) - if rec is None: - raise ToolNotFound(tool_id) - if not constant_time_eq(val, rec.esecret): + matched = constant_time_eq(val, rec.esecret if rec else "0" * 64) + if rec is None or not matched: raise AuthError(f"{_ES_PREFIX} mismatch") return rec @@ -154,6 +152,10 @@ def handle_register(ctx: HandlerContext, msg: dict[str, Any]) -> dict[str, Any]: if not isinstance(esecret, str) or not _HEX_RE.fullmatch(esecret) or len(esecret) < 32: raise BadRegistration("esecret must be a hex string of at least 32 chars") entries = parse_cs_tuples(msg.get("secrets")) + allowed = ctx.config.grants.get(tool_id, ()) + if any((entry["item"] or tool_id) != tool_id and entry["item"] not in allowed + for entry in entries): + raise AuthError("item is not granted to this tool") ctx.store.register(tool_id, Registration(esecret=esecret, secret_entries=entries)) return {"status": "ok"} @@ -184,8 +186,8 @@ def _get_secrets_bounded(ctx: HandlerContext, msg: dict[str, Any]) -> dict[str, item = entry.get("item") or tool_id try: value = ctx.plugin.get_secret(entry["field"], item) - except KeyError as exc: - raise BackendError(f"{tool_id}.{entry['name']}: {exc}") from exc + except Exception as exc: + raise BackendError("provider read failed") from exc body[entry["name"]] = value return {"status": "ok", "secrets": body} @@ -202,8 +204,8 @@ def handle_get_secret(ctx: HandlerContext, msg: dict[str, Any]) -> dict[str, Any item = match.get("item") or tool_id try: value = ctx.plugin.get_secret(match["field"], item) - except KeyError as exc: - raise BackendError(f"{tool_id}.{name}: {exc}") from exc + except Exception as exc: + raise BackendError("provider read failed") from exc return {"status": "ok", "secrets": {name: value}} @@ -221,6 +223,11 @@ def handle_write_secret(ctx: HandlerContext, msg: dict[str, Any]) -> dict[str, A value = msg.get("value") if not isinstance(value, str): raise BadRequest("'value' must be a string") + try: + if len(value.encode("utf-8")) > ctx.config.sp_max_value_bytes: + raise BadRequest("value exceeds size limit") + except UnicodeEncodeError: + raise BadRequest("value must be UTF-8") from None item = match.get("item") or tool_id try: ctx.plugin.write_secret(match["field"], item, value) diff --git a/sps/plugins/localfile.py b/sps/plugins/localfile.py index d63df03..2eae332 100644 --- a/sps/plugins/localfile.py +++ b/sps/plugins/localfile.py @@ -29,10 +29,10 @@ # --- Encryption layer (lifted from toolyard/secrets.py:90-176) ---------------- _VAULT_VERSION = 1 -_SCRYPT_N = 2 ** 14 # ~16 MB, tens of ms: interactive-grade stretching +_SCRYPT_N = 2 ** 16 # New vaults use ~64 MiB of memory. _SCRYPT_R = 8 _SCRYPT_P = 1 -_SCRYPT_MAXMEM = 64 * 1024 * 1024 # headroom for the params above (else scrypt errors) +_SCRYPT_MAXMEM = 128 * 1024 * 1024 # Includes implementation overhead. def _fernet(): diff --git a/sps/server.py b/sps/server.py index 5beff91..6b8aaee 100644 --- a/sps/server.py +++ b/sps/server.py @@ -19,12 +19,13 @@ import threading import hashlib import time +import select from dataclasses import dataclass from typing import Any from . import handlers from .audit import AuditLogger -from .config import Config +from .config import Config, validate_listener from .store import ToolRegistrationStore from .wire import ( OversizedBodyError, @@ -38,6 +39,31 @@ BODY_TIMEOUT = 5.0 # absolute JSON-line read budget, not reset by dribbled bytes +def handshake_with_deadline(request, timeout): + """Nonblocking TLS progress shares one absolute deadline.""" + deadline = time.monotonic() + timeout + request.setblocking(False) + try: + while True: + remaining = deadline - time.monotonic() + if remaining <= 0: + raise TimeoutError("TLS handshake deadline exceeded") + try: + request.do_handshake() + if time.monotonic() >= deadline: + raise TimeoutError("TLS handshake deadline exceeded") + return + except ssl.SSLWantReadError: + readers, writers = [request], [] + except ssl.SSLWantWriteError: + readers, writers = [], [request] + remaining = deadline - time.monotonic() + if remaining <= 0 or not any(select.select(readers, writers, [], remaining)[:2]): + raise TimeoutError("TLS handshake deadline exceeded") + finally: + request.settimeout(SOCKET_TIMEOUT) + + @dataclass class AppContext: config: Config @@ -117,8 +143,20 @@ def identity(key): response.get("message")) if outcome not in MESSAGE_SET and outcome != "ok": outcome = "Backend error" - self.ctx.audit.event(op, outcome=outcome, - tool_id=identity("toolid"), secret_name=identity("name")) + if op in ("ping", "invalid") or outcome in ("Bad request", "Unauthorized"): + self.server._record_reject("ping" if op == "ping" else "invalid") + else: + extra = {} + if op == "register" and outcome == "ok": + entries = msg["secrets"] + digest = lambda value: "sha256:" + hashlib.sha256(value.encode("utf-8")).hexdigest() + extra = {"entries": [{"field": digest(e["field"]), + "item": digest(e.get("item") or msg["toolid"])} + for e in entries], + "entry_count": len(entries), + "writable_count": sum(e.get("writable", False) for e in entries)} + self.ctx.audit.event(op, outcome=outcome, + tool_id=identity("toolid"), secret_name=identity("name"), **extra) except Exception: response = err_envelope("Backend error") try: @@ -137,7 +175,7 @@ def __init__(self, *args, max_workers=32, **kwargs): self.request_queue_size = max(self.request_queue_size, max_workers) self._slots = threading.BoundedSemaphore(max_workers) self._reject_lock = threading.Lock() - self._rejects = {"admission": 0, "transport": 0} + self._rejects = {"admission": 0, "transport": 0, "ping": 0, "invalid": 0} self._last_reject_audit = float("-inf") self.ssl_ctx = None super().__init__(*args, **kwargs) @@ -160,7 +198,7 @@ def process_request_thread(self, request, client_address): if self.ssl_ctx is not None: request = self.ssl_ctx.wrap_socket(request, server_side=True, do_handshake_on_connect=False) - request.do_handshake() + handshake_with_deadline(request, SOCKET_TIMEOUT) super().process_request_thread(request, client_address) except (OSError, ssl.SSLError): self._record_reject("transport") @@ -175,7 +213,7 @@ def _record_reject(self, kind): self._rejects[kind] = min(self._rejects[kind] + 1, 2**31 - 1) def _flush_rejects(self): - # Best-effort operational summaries, at most two writes/second across + # Best-effort operational summaries, at most four writes/second across # all workers. Counts saturate and may be lost on failure/exit; these # are not durable per-rejection application audit records. with self._reject_lock: @@ -207,6 +245,7 @@ def build_server(ctx: AppContext, host: str = "127.0.0.1", port: int = 0, Plaintext requires the explicit allow_plaintext_for_tests opt-in.""" if ssl_ctx is None and not allow_plaintext_for_tests: raise ValueError("SPS requires TLS; plaintext is test-only") + validate_listener(host, getattr(getattr(ctx, "config", None), "sp_allow_nonloopback", False)) if not issubclass(server_class, ThreadingTCPServer): raise ValueError("SPS requires bounded handshake worker server") class _Bound(_StreamHandler): diff --git a/sps/tests/test_adjudicated_defects.py b/sps/tests/test_adjudicated_defects.py index 251232f..9282786 100644 --- a/sps/tests/test_adjudicated_defects.py +++ b/sps/tests/test_adjudicated_defects.py @@ -116,10 +116,12 @@ def test_reply_timeout_outcome_and_disconnect_sanitization(self): handler.connection = mock.Mock() handler.wfile = mock.Mock() handler.ctx = SimpleNamespace(audit=mock.Mock(), boot_id="fake") + handler.server = mock.Mock() with mock.patch("sps.server.read_socket_json", side_effect=failure, return_value={"op": "ping"}), mock.patch.dict(server._HANDLER_BY_OP, {"ping": lambda *_: {"status": "ok", "message": "never-log"}}), mock.patch("sps.server.write_one_json", side_effect=BrokenPipeError("never-log")): handler.handle() self.assertEqual(handler.connection.settimeout.call_args.args, (server.SOCKET_TIMEOUT,)) - self.assertEqual(handler.ctx.audit.event.call_args.kwargs["outcome"], "ok" if failure is None else "Bad request") + handler.ctx.audit.event.assert_not_called() + handler.server._record_reject.assert_called_once_with("ping" if failure is None else "invalid") with mock.patch("sys.stderr", new_callable=io.StringIO) as stderr: server.ThreadingTCPServer.handle_error(None, None, None) self.assertEqual(stderr.getvalue(), "") @@ -130,7 +132,7 @@ def test_audit_rotation_is_bounded_private_and_failure_propagates(self): audit = AuditLogger(path, max_bytes=4096) for _ in range(200): audit.event("ping", tool_id="x" * 128) - self.assertEqual(set(p.name for p in Path(temp).iterdir()), {"audit", "audit.1"}) + self.assertEqual(set(p.name for p in Path(temp).iterdir()), {"audit", *(f"audit.{i}" for i in range(1, 6))}) for file in Path(temp).iterdir(): self.assertLessEqual(file.stat().st_size, 4096) self.assertEqual(file.stat().st_mode & 0o777, 0o600) diff --git a/sps/tests/test_client.py b/sps/tests/test_client.py index 4988c2d..0789a6e 100644 --- a/sps/tests/test_client.py +++ b/sps/tests/test_client.py @@ -56,7 +56,7 @@ def write_secret(self, field, item, value): class _FakeAudit: - def event(self, action, *, tool_id="", secret_name="", outcome="ok"): + def event(self, action, *, tool_id="", secret_name="", outcome="ok", **metadata): pass @@ -134,10 +134,10 @@ def test_write_secret_200(self): # The FakePlugin accepts writes silently; the SPS returns ok. es.write_secret("echo", "api_key", "V-new") - def test_not_found_propagates(self): + def test_unregistered_tool_is_unauthorized(self): es = SPSClient("127.0.0.1", self.srv.port, esecret="e" + "3" * 63, ca_file=self.ca) - with self.assertRaises(NotFound): + with self.assertRaises(AuthError): es.get_secrets("never-registered") def test_bad_ca_fails_closed(self): diff --git a/sps/tests/test_config.py b/sps/tests/test_config.py index 63f9c1a..d401a81 100644 --- a/sps/tests/test_config.py +++ b/sps/tests/test_config.py @@ -67,7 +67,7 @@ class PluginBlocks(unittest.TestCase): def test_infisical_block(self): f = tempfile.NamedTemporaryFile("w", delete=False) f.write( - 'SP_HOST = "h"\nSP_PORT = "1"\nSP_SECRET = "s"\n' + 'SP_HOST = "127.0.0.1"\nSP_PORT = "1"\nSP_SECRET = "s"\n' 'SP_TLS_CERT = "c"\nSP_TLS_KEY = "k"\nSP_TLS_CA = "a"\n' 'SP_PLUGIN = "infisical"\n\n' '[infisical]\n' @@ -84,7 +84,7 @@ def test_infisical_block(self): def test_vault_block(self): f = tempfile.NamedTemporaryFile("w", delete=False) f.write( - 'SP_HOST = "h"\nSP_PORT = "1"\nSP_SECRET = "s"\n' + 'SP_HOST = "127.0.0.1"\nSP_PORT = "1"\nSP_SECRET = "s"\n' 'SP_TLS_CERT = "c"\nSP_TLS_KEY = "k"\nSP_TLS_CA = "a"\n' 'SP_PLUGIN = "hashicorp_vault"\n\n' '[hashicorp_vault]\nURL = "https://v"\nTOKEN = "t"\nMOUNT = "kv"\n' @@ -99,7 +99,7 @@ def test_vault_block(self): def test_localfile_block(self): f = tempfile.NamedTemporaryFile("w", delete=False) f.write( - 'SP_HOST = "h"\nSP_PORT = "1"\nSP_SECRET = "s"\n' + 'SP_HOST = "127.0.0.1"\nSP_PORT = "1"\nSP_SECRET = "s"\n' 'SP_TLS_CERT = "c"\nSP_TLS_KEY = "k"\nSP_TLS_CA = "a"\n' 'SP_PLUGIN = "localfile"\n\n' '[localfile]\nVAULT_FILE = "/var/lib/sps.vault.json"\n' diff --git a/sps/tests/test_policy_limits.py b/sps/tests/test_policy_limits.py new file mode 100644 index 0000000..7470412 --- /dev/null +++ b/sps/tests/test_policy_limits.py @@ -0,0 +1,239 @@ +"""Offline policy, retention and resource-limit regressions.""" +import io +import json +import os +from pathlib import Path +import socket +import ssl +import tempfile +import threading +import time +from dataclasses import replace +from types import SimpleNamespace +import unittest +from unittest import mock +from urllib.error import HTTPError + +from sps import cli, handlers, server, transport +from sps.audit import AuditLogger +from sps.client import SPSClient +from sps.config import InfisicalBlock, VaultBlock, LocalFileBlock, load_config, validate_listener +from sps.plugins.hashicorp_vault import HashicorpVaultPlugin +from sps.plugins.localfile import _Vault, _vault_key, _write_vault +from sps.tests.test_register import _ctx, _reg_msg +from sps.tests.test_wire_end_to_end import _gen_self_signed, _tls_context +from sps.tool_sdk import SecretClient + + +def request(ctx, msg, instance=None): + handler = object.__new__(server._StreamHandler) + handler.connection = mock.Mock() + handler.wfile = io.BytesIO() + handler.ctx = ctx + handler.server = instance or mock.Mock() + with mock.patch("sps.server.read_socket_json", return_value=msg): + handler.handle() + return json.loads(handler.wfile.getvalue()) + + +class PolicyLimits(unittest.TestCase): + def test_registration_policy_and_hashed_audit(self): + ctx = _ctx(audit=mock.Mock()) + message = _reg_msg(secrets=[{"name": "name", "field": "private-field", "item": "other/path", "writable": True}]) + self.assertEqual(request(ctx, message)["message"], "Unauthorized") + self.assertIsNone(ctx.store.get("echo")) + ctx.config = replace(ctx.config, grants={"echo": ("other/path",)}) + self.assertEqual(request(ctx, message)["status"], "ok") + event = ctx.audit.event.call_args.kwargs + self.assertEqual(event["entry_count"], 1) + self.assertEqual(event["writable_count"], 1) + self.assertTrue(all(v.startswith("sha256:") for v in event["entries"][0].values())) + self.assertNotIn("private-field", repr(event)) + self.assertNotIn("other/path", repr(event)) + ctx.config = replace(ctx.config, grants={}) + self.assertEqual(request(ctx, message)["message"], "Unauthorized") + self.assertEqual(ctx.store.get("echo").secret_entries[0]["item"], "other/path") + for item in (None, "echo"): + self.assertEqual(request(ctx, _reg_msg(secrets=[{"name": "n", "field": "f", "item": item}]))["status"], "ok") + + def test_ping_and_garbage_volume_is_rate_bounded(self): + ctx = _ctx(audit=mock.Mock()) + ctx.boot_id = "fake" + instance = object.__new__(server.ThreadingTCPServer) + instance._reject_lock = threading.Lock() + instance._rejects = dict.fromkeys(("admission", "transport", "ping", "invalid"), 0) + instance._last_reject_audit = float("-inf") + instance.RequestHandlerClass = SimpleNamespace(ctx=ctx) + with mock.patch("sps.server.time.monotonic", return_value=1): + for _ in range(500): + for msg in ({"op": "ping"}, {"op": "garbage"}, [], {"op": "register"}): + request(ctx, msg, instance) + instance._flush_rejects() + self.assertLessEqual(ctx.audit.event.call_count, 2) + with mock.patch("sps.server.time.monotonic", return_value=2): + instance._flush_rejects() + self.assertEqual(sum(c.kwargs["count"] for c in ctx.audit.event.call_args_list), 2000) + self.assertTrue(all(c.kwargs["outcome"] == "rejected" for c in ctx.audit.event.call_args_list)) + + def test_audit_retains_generations_and_only_hashes(self): + with tempfile.TemporaryDirectory() as temp: + log = AuditLogger(Path(temp) / "audit", max_bytes=1024, generations=3) + for i in range(100): + log.event("register", tool_id=str(i)) + self.assertEqual(len(list(Path(temp).iterdir())), 4) + with self.assertRaises(ValueError): + log.event("register", entries=[{"field": "raw", "item": "raw"}], entry_count=1, writable_count=0) + + def test_tls_dribble_hits_absolute_deadline(self): + with tempfile.TemporaryDirectory() as temp: + cert, key = _gen_self_signed(Path(temp)) + tls = _tls_context(cert, key) + client = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + incoming, outgoing = ssl.MemoryBIO(), ssl.MemoryBIO() + peer = client.wrap_bio(incoming, outgoing, server_hostname="localhost") + with self.assertRaises(ssl.SSLWantReadError): + peer.do_handshake() + hello = outgoing.read() + left, right = socket.socketpair() + wrapped = tls.wrap_socket(left, server_side=True, do_handshake_on_connect=False) + stop = threading.Event() + sent = [] + def dribble(): + try: + for byte in hello: + if stop.wait(0.02): + return + right.sendall(bytes([byte])) + sent.append(byte) + except OSError: + pass + thread = threading.Thread(target=dribble) + thread.start() + start = time.monotonic() + try: + with self.assertRaises(TimeoutError): + server.handshake_with_deadline(wrapped, 0.25) + self.assertLess(time.monotonic() - start, 0.8) + self.assertGreater(len(sent), 3) + finally: + stop.set() + thread.join(2) + wrapped.close() + right.close() + + def test_bulk_http_error_closes_and_timeout_maps_to_backend(self): + ctx = _ctx(audit=mock.Mock()) + handlers.handle_register(ctx, _reg_msg()) + ctx.plugin = HashicorpVaultPlugin(VaultBlock("https://example.invalid", "fake")) + body = io.BytesIO(b"private-provider-body") + error = HTTPError("https://example.invalid", 403, "private", {}, body) + for failure in (error, TimeoutError("private")): + with mock.patch("sps.transport.urlopen", side_effect=failure): + result = request(ctx, {"op": "get_secrets", "toolid": "echo", "esecret": _reg_msg()["esecret"]}) + self.assertEqual(result, {"status": "error", "message": "Backend error"}) + self.assertIsNone(transport._active.get()) + self.assertTrue(body.closed) + + def test_unknown_tool_and_wrong_secret_have_identical_response(self): + ctx = _ctx(audit=mock.Mock()) + ctx.plugin.timeout = 15.0 + handlers.handle_register(ctx, _reg_msg()) + for op in ("get_secrets", "get_secret", "write_secret"): + results = [] + with mock.patch("sps.handlers.constant_time_eq", wraps=handlers.constant_time_eq) as compare: + for tool in ("echo", "unknown"): + results.append(request(ctx, {"op": op, "toolid": tool, "esecret": "wrong", "name": "api_key", "value": "x"})) + self.assertEqual(compare.call_count, 2) + self.assertEqual(results, [{"status": "error", "message": "Unauthorized"}] * 2) + + def test_credential_representations(self): + secret = "unique-private-credential" + objects = [replace(_ctx().config, sp_secret=secret), VaultBlock("https://example.invalid", secret), + InfisicalBlock("https://example.invalid", "vault", client_secret=secret), + SPSClient("localhost", 1, sp_secret=secret, esecret=secret)] + for obj in objects: + self.assertNotIn(secret, repr(obj)) + + def test_cli_file_passphrase(self): + with tempfile.TemporaryDirectory() as temp: + path = Path(temp) / "passphrase" + path.write_text("fake-passphrase\n") + path.chmod(0o600) + cfg = replace(_ctx().config, sp_plugin="localfile", localfile=LocalFileBlock("unused")) + args = SimpleNamespace(config="unused", tool="tool", field="field") + with mock.patch.dict(os.environ, {"SP_VAULT_PASSPHRASE_FILE": str(path)}, clear=True), mock.patch("sps.cli.cfgmod.load_config", return_value=cfg), mock.patch("sps.plugins.localfile._Vault") as vault, mock.patch("sys.stdin", io.StringIO("fake-value\n")), mock.patch("builtins.print"): + cli._cmd_vault_set(args) + cli._cmd_vault_get(args) + self.assertEqual(vault.call_count, 2) + self.assertTrue(all(c.args == ("unused", "fake-passphrase") for c in vault.call_args_list)) + path.chmod(0o644) + with self.assertRaises((PermissionError, ValueError)): + cli._cmd_vault_get(args) + + def test_write_value_utf8_byte_limit(self): + ctx = _ctx(audit=mock.Mock()) + handlers.handle_register(ctx, _reg_msg(secrets=[{"name": "key", "field": "f", "writable": True}])) + base = {"op": "write_secret", "toolid": "echo", "esecret": _reg_msg()["esecret"], "name": "key"} + for value in ("x" * 65536, "é" * 32768, ""): + self.assertEqual(request(ctx, {**base, "value": value})["status"], "ok") + ctx.plugin.reset_mock() + for value in ("x" * 65537, "é" * 32769, "\ud800"): + self.assertEqual(request(ctx, {**base, "value": value})["message"], "Bad request") + ctx.plugin.write_secret.assert_not_called() + ctx.config = replace(ctx.config, sp_max_value_bytes=2) + self.assertEqual(request(ctx, {**base, "value": "abc"})["message"], "Bad request") + + def test_config_grants_and_listener_opt_in(self): + for host in ("localhost", "127.0.0.1", "::1"): + validate_listener(host) + for host in ("0.0.0.0", "::", "example.invalid"): + with self.assertRaises(ValueError): + validate_listener(host) + validate_listener(host, True) + with tempfile.TemporaryDirectory() as temp: + path = Path(temp) / "sps.env" + base = 'SP_HOST="0.0.0.0"\nSP_PORT="1"\nSP_SECRET="fake"\nSP_PLUGIN="localfile"\n' + path.write_text(base) + path.chmod(0o600) + with self.assertRaises(ValueError): + load_config(path) + path.write_text(base + 'SP_ALLOW_NONLOOPBACK="1"\nSP_MAX_VALUE_BYTES=2048\n[grants]\necho=["shared/path"]\n') + cfg = load_config(path) + self.assertEqual(cfg.grants, {"echo": ("shared/path",)}) + self.assertEqual(cfg.sp_max_value_bytes, 2048) + for extra in ('SP_ALLOW_NONLOOPBACK="typo"', 'SP_ALLOW_NONLOOPBACK=true\nSP_MAX_VALUE_BYTES=0', 'SP_ALLOW_NONLOOPBACK=true\n[grants]\necho="*"'): + path.write_text(base + extra) + with self.assertRaises(ValueError): + load_config(path) + + def test_new_and_legacy_vault_parameters(self): + with tempfile.TemporaryDirectory() as temp: + new, old = Path(temp) / "new", Path(temp) / "old" + _Vault.init(new, "fake-pass") + self.assertEqual(json.loads(new.read_text())["n"], 2 ** 16) + salt = b"0123456789abcdef" + params = (2 ** 14, 8, 1) + _write_vault(old, salt, _vault_key("fake-pass", salt, *params), {"tool": {"f": "old"}}, params) + vault = _Vault(old, "fake-pass") + self.assertEqual(vault.get("tool", "f"), "old") + vault.set("tool", "f", "new") + self.assertEqual(json.loads(old.read_text())["n"], 2 ** 14) + + def test_loopback_certificate_lifetime_and_warning(self): + with tempfile.TemporaryDirectory() as temp: + cert, key, ca = (str(Path(temp) / name) for name in ("cert", "key", "ca")) + cli.maybe_generate_tls_material(cert, key, ca, "/CN=127.0.0.1") + expires = ssl.cert_time_to_seconds(ssl._ssl._test_decode_cert(cert)["notAfter"]) + self.assertGreater(expires - time.time(), 3000 * 86400) + for days, expected in ((31, False), (29, True), (-1, True)): + with mock.patch("time.time", return_value=expires - days * 86400), mock.patch("sys.stderr", io.StringIO()) as err: + cli.warn_certificate_expiry(cert) + self.assertEqual(bool(err.getvalue()), expected) + + def test_fake_sdk_refuses_present_credentials(self): + for credential in ("fake", ""): + with mock.patch.dict(os.environ, {"TOOLSTACK_SPS_FAKE": "1", "TOOLSTACK_E_SECRET": credential}, clear=True): + with self.assertRaises(RuntimeError): + SecretClient.from_env("echo") + with mock.patch.dict(os.environ, {"TOOLSTACK_SPS_FAKE": "1"}, clear=True): + self.assertEqual(SecretClient.from_env("echo").names(), ()) diff --git a/sps/tests/test_secrets_get.py b/sps/tests/test_secrets_get.py index 05fcd89..ccf77f7 100644 --- a/sps/tests/test_secrets_get.py +++ b/sps/tests/test_secrets_get.py @@ -74,7 +74,7 @@ def test_rejects_wrong_es_secret(self): def test_unknown_tool(self): ctx = _registered_ctx() - with self.assertRaises(ToolNotFound): + with self.assertRaises(AuthError): handle_get_secrets(ctx, {"op": "get_secrets", "toolid": "nope", "esecret": ESECRET}) diff --git a/sps/tests/test_update.py b/sps/tests/test_update.py index 4453dfe..ab9a41f 100644 --- a/sps/tests/test_update.py +++ b/sps/tests/test_update.py @@ -65,7 +65,7 @@ def test_success(self): def test_unknown_tool(self): ctx = _ctx() - with self.assertRaises(ToolNotFound): + with self.assertRaises(AuthError): handle_write_secret(ctx, { "op": "write_secret", "toolid": "nope", "esecret": ESECRET, "name": "api_key", "value": "x", diff --git a/sps/tests/test_uplift_security.py b/sps/tests/test_uplift_security.py index 8d425fb..04a0b82 100644 --- a/sps/tests/test_uplift_security.py +++ b/sps/tests/test_uplift_security.py @@ -54,7 +54,8 @@ def dribble(conn): response = read_one_json(conn.makefile("rb")) self.assertEqual(response["message"], "Bad request") self.assertLess(time.monotonic() - started, 1.5) - audit.event.assert_called_with("invalid", outcome="Bad request", tool_id="", secret_name="") + self.assertFalse(any(call.kwargs.get("outcome") == "Bad request" + for call in audit.event.call_args_list)) finally: stop.set() sender.join(2) @@ -228,18 +229,18 @@ def test_rejects_and_audit_failure_before_reply(self): with socket.create_connection(server.server_address) as conn: conn.sendall(body) self.assertEqual(read_one_json(conn.makefile("rb"))["message"], "Bad request") - records = Path(temp, "audit").read_text() + records = Path(temp, "audit").read_text() if Path(temp, "audit").exists() else "" self.assertNotIn("never-log", records) with socket.create_connection(server.server_address) as conn: conn.sendall(b'{"op":"register","spsecret":"never-log","toolid":"fake-tool"}\n') self.assertEqual(read_one_json(conn.makefile("rb"))["message"], "Unauthorized") - records = Path(temp, "audit").read_text() - self.assertIn("Unauthorized", records) + records = Path(temp, "audit").read_text() if Path(temp, "audit").exists() else "" + self.assertNotIn("Unauthorized", records) self.assertNotIn("never-log", records) self.assertNotIn("fake-tool", records) with mock.patch.object(audit, "event", side_effect=OSError("disk full")): with socket.create_connection(server.server_address) as conn: - conn.sendall(b'{"op":"ping"}\n') + conn.sendall(b'{"op":"unregister","spsecret":"fake-master","toolid":"fake-tool"}\n') self.assertEqual(read_one_json(conn.makefile("rb"))["message"], "Backend error") finally: server.shutdown() diff --git a/sps/tests/test_wire_end_to_end.py b/sps/tests/test_wire_end_to_end.py index 63e644a..7fb0244 100644 --- a/sps/tests/test_wire_end_to_end.py +++ b/sps/tests/test_wire_end_to_end.py @@ -47,7 +47,7 @@ class _FakeAudit: def __init__(self): self.events: list[tuple[str, str, str]] = [] - def event(self, action, *, tool_id="", secret_name="", outcome="ok"): + def event(self, action, *, tool_id="", secret_name="", outcome="ok", **metadata): self.events.append((action, tool_id, secret_name)) diff --git a/sps/tool_sdk.py b/sps/tool_sdk.py index 7da23e5..89fd86d 100644 --- a/sps/tool_sdk.py +++ b/sps/tool_sdk.py @@ -44,6 +44,8 @@ def __init__(self, tool_id: str, *, host: str, port: int, @classmethod def from_env(cls, tool_id: str) -> "SecretClient": if os.environ.get("TOOLSTACK_SPS_FAKE") == "1": + if "TOOLSTACK_E_SECRET" in os.environ: + raise RuntimeError("fake SPS cannot be used with TOOLSTACK_E_SECRET") # Test fixture: skip the SPS roundtrip. Used by toolyard/tests so the # tool can boot without a real server. Production never sets this. obj = cls.__new__(cls) diff --git a/sps/transport.py b/sps/transport.py index 053902a..02c8544 100644 --- a/sps/transport.py +++ b/sps/transport.py @@ -12,6 +12,7 @@ import ssl import time import urllib.request +import urllib.error MAX_RESPONSE = 8 * 1024 * 1024 _active = contextvars.ContextVar("sps_budget", default=None) @@ -46,19 +47,21 @@ def count(self, size): def operation(fn): @functools.wraps(fn) def call(self, *args, **kwargs): - if _active.get() is not None: - return fn(self, *args, **kwargs) - token = _active.set(Budget(self.timeout)) + token = None if _active.get() is not None else _active.set(Budget(self.timeout)) try: return fn(self, *args, **kwargs) except KeyError: raise KeyError("SPS provider value unavailable") from None except ValueError: raise ValueError("SPS provider invalid response") from None + except urllib.error.HTTPError as exc: + exc.close() + raise RuntimeError("SPS provider operation failed") from None except Exception: raise RuntimeError("SPS provider operation failed; mutation outcome may be unknown") from None finally: - _active.reset(token) + if token is not None: + _active.reset(token) return call