fix: confine PDP checks to the admitted local transport
Assistant: codex Assistant-Model: gpt-6-astra Assistant-Session: 01a12d13-27f2-7163-b8c0-f682af02772c
This commit is contained in:
parent
0db6adef69
commit
d379a3dfba
6 changed files with 268 additions and 50 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
],
|
||||
)
|
||||
|
|
|
|||
118
tests/test_decision_transport.py
Normal file
118
tests/test_decision_transport.py
Normal file
|
|
@ -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={})
|
||||
|
|
@ -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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue