diff --git a/docs/pdp-access-path.md b/docs/pdp-access-path.md index 99f373d..e599fe2 100644 --- a/docs/pdp-access-path.md +++ b/docs/pdp-access-path.md @@ -23,21 +23,40 @@ $ getent hosts this-service-does-not-exist.flex-auth.svc.cluster.local `*.svc.cluster.local` name answers with one unrelated public address. Sending there ships the CheckRequest body and our bearer token to a third party. -`flex-auth.decision-record.v1` carries **no signature**, and every digest in it -is either sent by us or published in flex-auth's repo, so a forger reproduces -all three exactly. Digests establish integrity of the binding, never +`flex-auth.decision-record.v1` supports a detached Ed25519 signature on the +producer side (FLEX-WP-0024), but this consumer does not yet verify it against +independently admitted keys. Without that verification, every binding digest +is either sent by us or published in flex-auth's repo, so a forger can +reproduce those digests. Digests establish integrity of the binding, never authenticity of the source. As flex-auth put it in `FLEX-DEC-2026-010`: > Fail-closed protects against a PDP that is ABSENT, not against one that LIES. -Until detached signatures ship (`FLEX-WP-0024`), the address shape is the only -thing standing in for responder authenticity. That is why it is enforced. +The literal loopback address restriction limits the destination; it does not +prove responder identity. The operator must verify the cluster and named pod +behind the port-forward. Signed consumer enforcement, independently admitted +verification keys, rotation and downgrade tests remain open in SECRETS-WP-0014 +and Custodian G13. Producer support alone does not close that gap. + +The Check client disables automatic redirects, environment/system proxies and +the process-global urllib opener. A 3xx response fails closed without contacting +the redirect destination. URLs must be HTTP(S) bare origins with a literal +loopback IP and optional nonzero port: no hostname (including `localhost`), +userinfo, path prefix, query, fragment or whitespace/control characters. Existing +`localhost` configuration must use `127.0.0.1` or `[::1]`. Invalid configuration +is rejected before reading the bearer file. Responses are limited to 512 KiB; +oversized, invalid UTF-8 and non-object JSON bodies fail closed. These rules also +apply when the approval flow invokes the PDP client. + +Regression tests use disposable local HTTP servers and synthetic credentials; +they do not demonstrate identity or signature verification on the native pin. ## The supported path -Loopback `kubectl port-forward` to the named pod. It resolves no DNS name, -targets one pod explicitly, and rides the Kubernetes API server's TLS with our -cluster credentials — so it authenticates the responder transitively. Note this +Use literal loopback `kubectl port-forward` to the admitted named pod after +verifying the cluster identity and pod identity. That path rides the Kubernetes +API server's TLS with the operator's cluster credentials; the client cannot infer +that binding from its local URL. Note this is the exact reverse of the caller direction, where port-forward bypasses the NetworkPolicy (`FLEX-WP-0023`); the two properties point opposite ways and summarising either as "the network protects it" gets one backwards. diff --git a/src/secrets_engine/approval_consume.py b/src/secrets_engine/approval_consume.py index cfe1aac..382d83c 100644 --- a/src/secrets_engine/approval_consume.py +++ b/src/secrets_engine/approval_consume.py @@ -16,7 +16,7 @@ from dataclasses import dataclass, field from pathlib import Path from typing import Any, Callable from urllib.error import HTTPError, URLError -from urllib.request import Request, urlopen +from urllib.request import Request from secrets_engine.approval_auth import approval_auth_configured, approval_token, credential_urlopen from secrets_engine.execution_contract import ExecutionContract, require_execution_deadline @@ -332,7 +332,7 @@ def authorize_action( base_url=pdp_url, token_file=Path(pdp_token), request=expected_request, - opener=pdp_opener or urlopen, + opener=pdp_opener, ) if not getattr(cfg, "clock_trust_file", None): wait_for_decision_start(envelope) diff --git a/src/secrets_engine/decision_check.py b/src/secrets_engine/decision_check.py index 252d22b..396f430 100644 --- a/src/secrets_engine/decision_check.py +++ b/src/secrets_engine/decision_check.py @@ -16,7 +16,7 @@ from pathlib import Path from urllib.parse import urlsplit from typing import Any, Callable from urllib.error import HTTPError, URLError -from urllib.request import Request, urlopen +from urllib.request import HTTPRedirectHandler, ProxyHandler, Request, build_opener from secrets_engine.errors import DecisionError from secrets_engine.openbao import read_strict_token_file @@ -28,8 +28,6 @@ _CLUSTER_SUFFIXES = (".svc.cluster.local", ".svc", ".cluster.local") def _is_loopback(host: str) -> bool: - if host in ("localhost", "localhost."): - return True try: return ipaddress.ip_address(host.strip("[]")).is_loopback except ValueError: @@ -37,47 +35,51 @@ def _is_loopback(host: str) -> bool: def require_supported_pdp_address(base_url: str) -> None: - """Refuse address shapes the owner path excludes (FLEX-DEC-2026-010). + """Require a literal loopback origin before reading the caller credential. - The supported operator path is a loopback ``kubectl port-forward`` over the - authenticated Kubernetes API, which is the only shape that authenticates the - *responder*: it resolves no DNS name, targets one named pod, and rides the - API server's TLS with our cluster credentials. - - A cluster Service name must never be dialled from a workstation. It does not - merely fail -- it RESOLVES, to the wrong host. This workstation carries - ``search ad.binect.de``, a wildcard zone, so every ``*.svc.cluster.local`` - name (including services that do not exist) answers with one unrelated - public address. Sending there would ship the CheckRequest body and our - bearer token to a third party, and because ``flex-auth.decision-record.v1`` - carries no signature, a responder that knows the published package id and - version can return a well-formed allow that passes every check we make. - - Fail-closed protects against a PDP that is ABSENT, not one that LIES. Until - FLEX-WP-0024 ships detached signatures, the address shape is the only thing - standing in for responder authenticity, so it is enforced rather than - documented. + The operator must independently bind the port-forward to the admitted + Kubernetes cluster and named PDP pod. URL validation alone cannot establish + the identity of a process listening on a local port. Hostnames, redirects + and ambient proxies must not redirect the credential away from that port. """ - host = (urlsplit(base_url).hostname or "").rstrip(".").lower() - if not host: - raise DecisionError("access-engine check URL has no host") - if any(host.endswith(suffix) for suffix in _CLUSTER_SUFFIXES): + try: + url = urlsplit(base_url) + host = (url.hostname or "").lower() + valid = ( + bool(host) and url.scheme in {"http", "https"} + and url.username is None and url.password is None + and not url.query and not url.fragment and url.path in {"", "/"} + and url.port != 0 + and not any(ord(char) <= 32 or ord(char) == 127 for char in base_url) + ) + except ValueError: + valid = False + if not valid: + raise DecisionError("access-engine check URL is missing or invalid") + if any(host.rstrip(".").endswith(suffix) for suffix in _CLUSTER_SUFFIXES): raise DecisionError( - f"access-engine check URL '{host}' is an in-cluster Service name; " - "from a workstation it resolves through the DNS search suffix to an " - "unrelated public host. Use the owner-documented loopback " - "kubectl port-forward instead (docs/pdp-access-path.md)" + "access-engine check URL is an in-cluster Service name; use the " + "owner-bound literal loopback port-forward (docs/pdp-access-path.md)" ) if not _is_loopback(host): raise DecisionError( - f"access-engine check URL host '{host}' is not loopback; the " - "supported path is a loopback kubectl port-forward over the " - "authenticated Kubernetes API, which is what authenticates the " - "responder (FLEX-DEC-2026-010). Set SECRETS_ENGINE_PDP_URL to the " - "forwarded 127.0.0.1 port (docs/pdp-access-path.md)" + "access-engine check URL host is not loopback in literal IP form; " + "use the owner-bound 127.0.0.1 or [::1] port-forward " + "(docs/pdp-access-path.md)" ) +class _NoRedirect(HTTPRedirectHandler): + def redirect_request(self, req, fp, code, msg, headers, newurl): + return None + + +def _pdp_urlopen(request, *, timeout): + # This is a local tunnel: never inherit an environment/system proxy or a + # process-global opener. No redirect, including same-origin, is a decision. + return build_opener(ProxyHandler({}), _NoRedirect()).open(request, timeout=timeout) + + def _status_message(status: int) -> str: if status in (401, 403): return "access-engine refused the caller" @@ -94,7 +96,7 @@ def check_decision( token_file: Path, request: dict[str, Any], timeout_seconds: float = 3, - opener: Callable[..., Any] = urlopen, + opener: Callable[..., Any] | None = None, ) -> dict[str, Any]: """POST /v1/check. Any non-200, non-JSON, or transport failure fails closed. @@ -115,15 +117,18 @@ def check_decision( http_request.add_header("Content-Type", "application/json") http_request.add_header("Accept", "application/json") try: - with opener(http_request, timeout=timeout_seconds) as response: + with (opener or _pdp_urlopen)(http_request, timeout=timeout_seconds) as response: if getattr(response, "status", 200) != 200: raise DecisionError("access-engine check did not return a decision") - payload = json.loads(response.read(_MAX_BODY).decode("utf-8")) + body = response.read(_MAX_BODY + 1) + if len(body) > _MAX_BODY: + raise DecisionError("access-engine check body is too large") + payload = json.loads(body.decode("utf-8")) except HTTPError as e: raise DecisionError(f"access-engine check refused: {_status_message(e.code)}") from e except URLError as e: raise DecisionError("access-engine is unreachable for check") from e - except json.JSONDecodeError as e: + except (json.JSONDecodeError, UnicodeDecodeError) as e: raise DecisionError("access-engine check returned a non-JSON body") from e if not isinstance(payload, dict): raise DecisionError("access-engine check returned a non-object body") diff --git a/tests/test_decision_replay.py b/tests/test_decision_replay.py index 8e93af7..3dcfaac 100644 --- a/tests/test_decision_replay.py +++ b/tests/test_decision_replay.py @@ -161,7 +161,6 @@ def test_unsupported_pdp_addresses_are_refused(url, fragment): "url", [ "http://127.0.0.1:18080", - "http://localhost:18080", "http://[::1]:18080", ], ) diff --git a/tests/test_decision_transport.py b/tests/test_decision_transport.py new file mode 100644 index 0000000..4bab910 --- /dev/null +++ b/tests/test_decision_transport.py @@ -0,0 +1,118 @@ +"""Exercise the real HTTP boundary with synthetic credentials and local servers.""" +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from threading import Thread +from types import SimpleNamespace + +import pytest + +from secrets_engine import approval_consume +from secrets_engine.decision_check import check_decision +from secrets_engine.errors import DecisionError + + +@contextmanager +def server(*, status=200, body=b'{}', location=None): + seen = [] + + class Handler(BaseHTTPRequestHandler): + def handle_request(self): + # Record only a boolean about our synthetic credential. + seen.append(self.headers.get('Authorization') == 'Bearer synthetic-pdp-credential') + self.rfile.read(int(self.headers.get('Content-Length', '0'))) + self.send_response(status) + if location: + self.send_header('Location', location) + self.send_header('Content-Length', str(len(body))) + self.end_headers() + self.wfile.write(body) + + do_POST = do_GET = handle_request + + def log_message(self, *_args): + pass + + httpd = ThreadingHTTPServer(('127.0.0.1', 0), Handler) + thread = Thread(target=httpd.serve_forever, kwargs={'poll_interval': 0.01}, daemon=True) + thread.start() + try: + yield f'http://127.0.0.1:{httpd.server_port}', seen + finally: + httpd.shutdown() + httpd.server_close() + thread.join(timeout=2) + + +@pytest.fixture +def credential(tmp_path): + path = tmp_path / 'synthetic.token' + path.write_text('synthetic-pdp-credential') + path.chmod(0o600) + return path + + +@pytest.fixture(autouse=True) +def clear_proxies(monkeypatch): + # Tests must not contact proxies configured on the developer's machine. + import os + import urllib.request + monkeypatch.setattr(urllib.request, "_opener", None) + for name in os.environ: + if name.lower().endswith('_proxy'): + monkeypatch.delenv(name) + + +@pytest.mark.parametrize('status', [301, 302, 303, 307, 308]) +def test_redirect_never_contacts_another_origin(credential, status): + with server() as (destination, received): + with server(status=status, location=destination + '/sink') as (source, sent): + with pytest.raises(DecisionError): + check_decision(base_url=source, token_file=credential, request={}) + assert sent == [True] + assert received == [] + + +def test_environment_proxy_never_receives_request(credential, monkeypatch): + with server() as (proxy, proxied): + with server() as (source, direct): + monkeypatch.setenv('http_proxy', proxy) + monkeypatch.setenv('no_proxy', '') + assert check_decision(base_url=source, token_file=credential, request={}) == {} + assert direct == [True] + assert proxied == [] + + +def test_approval_flow_uses_the_same_safe_transport(credential, monkeypatch): + observation = SimpleNamespace(request={}) + monkeypatch.setattr(approval_consume, 'resolve_approval_observation', lambda *a, **kw: observation) + with server() as (destination, received): + with server(status=302, location=destination + '/sink') as (source, sent): + cfg = SimpleNamespace(pdp_url=source, pdp_token_file=credential, + authorization_policy_package='test', authorization_policy_version='v1') + with pytest.raises(DecisionError): + approval_consume.authorize_action(cfg, object(), 'rotate', None) + assert sent == [True] + assert received == [] + + +@pytest.mark.parametrize('url', [ + 'http://localhost:8080', 'http://localhost.:8080', + 'http://user:synthetic-password@127.0.0.1:8080', + 'http://127.0.0.1:8080?query=value', 'http://127.0.0.1:8080#fragment', + 'http://127.0.0.1:8080/path', 'http://127.0.0.1:0', + 'http://127.0.0.1:wrong', 'http://[::1', 'http://127.0.0.1:65536', + 'http://127.0.0.1:\n8080', ' http://127.0.0.1:8080', 'file://127.0.0.1', +]) +def test_invalid_address_refused_before_credential_read(url, tmp_path): + with pytest.raises(DecisionError, match='URL'): + check_decision(base_url=url, token_file=tmp_path / 'missing', request={}) + + +@pytest.mark.parametrize('body,reason', [ + (b'{}' + b' ' * (512 * 1024), 'too large'), + (b'\xff', 'non-JSON'), (b'[]', 'non-object'), +], ids=['oversized', 'invalid-utf8', 'array']) +def test_response_limits_and_encoding_fail_closed(credential, body, reason): + with server(body=body) as (source, _): + with pytest.raises(DecisionError, match=reason): + check_decision(base_url=source, token_file=credential, request={}) diff --git a/workplans/SECRETS-WP-0014-pdp-transport-and-responder-authentication.md b/workplans/SECRETS-WP-0014-pdp-transport-and-responder-authentication.md new file mode 100644 index 0000000..ad06eb0 --- /dev/null +++ b/workplans/SECRETS-WP-0014-pdp-transport-and-responder-authentication.md @@ -0,0 +1,77 @@ +--- +id: SECRETS-WP-0014 +type: workplan +title: "Constrain PDP transport and authenticate decision responses" +domain: infotech +repo: secrets-engine +status: active +owner: codex +topic_slug: secrets-engine +created: "2026-10-12" +updated: "2026-10-12" +flavor: implementation +depends_on: [] +state_hub_workstream_id: "7252469d-d7a5-5c18-8911-b511b8c6a736" +--- + +Native owner work for CUST-WP-0080 G13. Reuses the accepted Check contract and +FLEX-WP-0024 producer signature surface; does not create another PDP or grant +any catalog-lane execution authority. SECRETS-WP-0013 still owns task-scoped +protected execution. Transport review is complete; key admission and native +rollout remain separate gates below. + +## Prevent the Check credential from leaving the admitted local transport + +```task +id: SECRETS-WP-0014-T01 +status: done +priority: high +state_hub_task_id: "8baba9a3-3513-5f02-bafe-ed9dce743173" +``` + +Reproduced redirects (301/302/303), environment proxy forwarding, malformed URL +handling, oversized-body truncation and invalid UTF-8 against the prior source. +The bounded fix disables redirects and ambient proxies/global openers, requires +a literal loopback bare origin before credential access, caps the complete +response body and rejects invalid UTF-8 as a DecisionError. The approval consumer +no longer overrides this safe default with urlopen. Synthetic local-server tests +cover both entry points, all five redirect status codes and proxy forwarding. +No real credentials or protected backend were touched. Source verification is +recorded in the Custodian evidence directory for 2026-10-12, pdp-transport.json. +This closes the source transport defects only; T02/T03 remain mandatory. + +## Implement independently anchored responder authentication + +```task +id: SECRETS-WP-0014-T02 +status: todo +priority: high +state_hub_task_id: "877782e5-045b-5186-8b95-82e25db21a5a" +``` + +Review producer canonical signature bytes and the native pin's unsigned/signed +mode. Establish independently admitted verification keys and exact issuer/pin +binding with AccessEngine and Platform; an unauthenticated keys endpoint on the +same responder is not a trust anchor. Reuse the native producer fixtures rather +than inventing a Python canonicalization. Require valid signed envelopes on the +production consumer path; prove forged allow, altered payload, unknown key, +wrong issuer/pin, unsigned/downgrade, expired and rotation overlap/refusal cases. +Track any temporary exception using all required SEC-DSM-EXC fields and actual +approval; neither source support nor a loopback URL is that approval. + +## Verify the admitted native consumer and rotation path + +```task +id: SECRETS-WP-0014-T03 +status: wait +priority: high +depends_on: [SECRETS-WP-0014-T02] +blocking_reason: "Requires reviewed verification-key admission, source enforcement and exact native rollout receipt; no production trust change is proposed yet." +state_hub_task_id: "85aa4c78-b66d-5201-8e31-0ed50e7eaeec" +``` + +Under an exact reviewed rollout, prove the real pin and consumer accept the +admitted signer, reject forgery/downgrade, rotate without accepting removed keys, +and preserve request/approval binding, lifetime and single-use consumption. +Retain rollback and failure evidence. Update CUST-WP-0080 G13 only with these +native receipts; do not equate passing transport tests with closure.