SPS security hardening

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"]

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

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,38 +13,44 @@
 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
         # pass ephemeral paths we want creation to fail loudly rather than
         # 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]
         if secret_name:
             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

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")
 

sps/client.py

@@ -61,6 +61,9 @@ def _default_context(**kwargs):
 
 
 class SPSClient:
+    def __repr__(self) -> str:
+        return "SPSClient(credentials=<redacted>)"
+
     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,

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,
     )

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)