From 999e71deec30e3f6f202617767f8896b703c54f7 Mon Sep 17 00:00:00 2001 From: phoenix-server Date: Thu, 17 Sep 2026 16:30:28 -0400 Subject: [PATCH 1/2] authorize-docker-requests: match paths without the API version prefix (#35) The plugin's PathPlain is the raw request path, version prefix included (main.go makeInput: "PathPlain": u.Path), so every rule keyed on that field matched nothing on a live daemon: the R-17 equality grants and the startswith rules behind lifecycle, container delete and path-named network/volume calls. The sandbox could not create, start, stop or delete anything for its own project. It failed closed, which is why it looked secure rather than broken, and the probe table agreed because its inputs were built from the documented shape - which was itself wrong. New requirement R-19: path matching is version-independent. The policy derives `path` (PathPlain with one optional /v[.] prefix removed, and no .. segment) and `path_segments` from it; PathArr is no longer read. An unversioned client keeps working, /_ping is untouched, a double prefix strips once, and a traversal path matches nothing. Evidence, in the plugin's real input shape (Path/PathPlain versioned, PathArr split from that, Query parsed), 78 rows on OPA 0.60.0, 1.3.0 and 1.7.1: the 0.4.0 policy decided wrongly on 26 rows - all legitimate requests denied - and this policy decides all 78 as specified, with identical decisions when the table is run without a version prefix and with v1.43. Docs: the Rego input contract corrected in SCHEMATIC.md, agent.rego.schema and modules/opa-policy.md, with input.Query documented and the derivation quoted; A-15 now requires the real shape and says a table alone is not enough; Limitations records a table is only as real as its inputs, and that a project name which is also an API path segment widens the segment match. Q-4 answered, and the reload procedure was wrong twice: disabling a plugin a running daemon references is fatal to the daemon (Error validating authorization plugin ... not found), and the bounce was never needed - the plugin reads the policy file on every request, so the deployed file is the live policy. The script now replaces the file by temp-and-rename (a missing policy file is the plugin's one fail-open path), touches no plugin state, and is verified through the decision log's config_hash. Q-5 answered yes: a snap daemon does bind host P-6 into the plugin at /opa. Closes #35 --- .../authorize-docker-requests/SCHEMATIC.md | 290 +++++++++++++++--- .../modules/opa-policy.md | 34 +- .../modules/policy-reload.md | 135 +++++--- .../scripts/reload-opa-policy.sh | 220 +++++++++---- .../skeleton/agent.rego | 84 +++-- .../skeleton/agent.rego.schema | 70 ++++- 6 files changed, 636 insertions(+), 197 deletions(-) diff --git a/schematics/authorize-docker-requests/SCHEMATIC.md b/schematics/authorize-docker-requests/SCHEMATIC.md index cfa277c..ec0d7cd 100644 --- a/schematics/authorize-docker-requests/SCHEMATIC.md +++ b/schematics/authorize-docker-requests/SCHEMATIC.md @@ -1,7 +1,7 @@ --- name: authorize-docker-requests -version: 0.4.0 +version: 0.5.0 status: draft spec: 1 description: Grants a sandbox container restricted Docker daemon access over TLS, policed by Open Policy Agent — certificate infrastructure, Rego policy, systemd TCP listener, and sandbox client provisioning. @@ -40,6 +40,17 @@ updated: 2026-09-17 > table in which 30 of its 71 rows decided wrongly before the change and every > row decides as specified after it, on the three OPA engines the package names > (see `skeleton/agent.rego.schema`). +> +> **2026-09-17 — the path contract was wrong, and the hardening was dead on a +> live daemon.** `PathPlain` carries the API version prefix (`"PathPlain": +> u.Path` in the plugin's `main.go`), so every rule the hardening keyed on that +> field by equality — the create, build, pull, volume and network grants — +> matched nothing on a real host, and the sandbox could not create anything for +> its own project. It failed **closed**, which is why the probe table did not +> catch it: the table was the only test, and its inputs were the documented +> shape, which was itself wrong. The policy now derives a version-free path +> (R-19) and the table's inputs are the plugin's real ones; see `main.go`'s +> `makeInput` for the two fields that differ. Grants a sandbox container (isolated agent, CI runner, or untrusted workload) restricted TCP access to the host Docker daemon, policed by Open Policy Agent. @@ -139,7 +150,8 @@ Limitations). host directory later bind-mounted into the container. - The integration step that makes the policy effective for the sandbox: removing its Docker socket mount (R-14). -- Policy reload procedure (bounce the plugin, not the daemon). +- Policy reload procedure (replace the policy file; nothing is bounced — see +the policy-reload module). - Verification commands for TLS connectivity and OPA enforcement. **Out of scope / non-goals:** @@ -209,11 +221,23 @@ deployment that does not accept them is deploying something else. outage or an open door, depending on how it fails**: unresolved placeholders make every request allowed (R-13), while a policy that does not compile leaves the daemon failing closed. -- **Verification is a build-time activity.** The policy is only proven by - running engines and probes as described in `skeleton/agent.rego.schema`; this - package's own pass ran its deny-probe table — 71 rows, decision-checked on - three OPA engines — under the engines the two installable plugin releases - embed (OPA v0.60.0 and v1.3.0) plus a newer one (v1.7.1). +- **A probe table can only be as real as its inputs.** The policy is proven by + running engines and probes as described in `skeleton/agent.rego.schema` — 78 + rows, decision-checked on three OPA engines (OPA v0.60.0 and v1.3.0, the two + the installable plugin releases embed, plus v1.7.1) — but a row decides only + about the input it carries. The 71-row table that shipped in 0.4.0 used a + version-less `PathPlain`, which the plugin never sends, so it passed while + every create on a live daemon was denied (issue #35). The rows now carry the + plugin's real values; only a **live** run (A-11, A-16) can find a mismatch + between the table's model of the input and the plugin's. +- **A project name that is also an API path segment widens the segment match.** + `P-3` is matched as a whole path segment (R-12, R-18). A deployment whose + project is called `containers` — the host this package was verified against — + therefore also satisfies the match on any path that contains the API's own + `containers` segment, so a foreign network or volume literally named + `containers` could be deleted. Container paths are unaffected in practice + (container delete and lifecycle calls are unscoped anyway). Pick a project + name that is not an API segment if that matters. **Preservation List** *(reverse-engineered; corrected 2026-09-16)*: @@ -305,6 +329,14 @@ deployment that does not accept them is deploying something else. start, stop, restart, kill, pause, unpause, wait, update. - **R-9**: The sandbox container MUST NOT require Docker configuration changes when the OPA policy is updated — policy reload is a host-side operation. + **Revision 2026-09-17**: "reload" is now precisely stated, because the + procedure this package shipped was wrong twice. The plugin re-reads the + policy **file on every request** (`os.ReadFile(p.policyFile)` inside + `evaluatePolicyFile`), so a policy change is a file replacement and nothing + else: no plugin bounce (disabling a plugin the daemon references makes + dockerd **exit** — measured, Q-4), no daemon restart, and no sandbox change. + The replacement must be atomic, because a *missing* policy file is the + plugin's one fail-open path. See the policy-reload module. - **R-10**: The host's unix socket (`/var/run/docker.sock`) MUST remain unrestricted for local host users. - **R-11**: The OPA policy MUST allow testcontainers-go containers — those @@ -373,6 +405,18 @@ deployment that does not accept them is deploying something else. `P-3_` as a whole path segment) MUST be allowed, and a delete of any other network or volume — a foreign name, or an opaque id — MUST be denied. Container deletion stays unscoped (R-5's revision; see Limitations). +- **R-19**: The OPA policy MUST match request paths independently of the API + version prefix, and MUST NOT depend on a field value the plugin does not + send. `PathPlain` is the raw request path **including** `/v.` + (`"PathPlain": u.Path` in the plugin's `main.go`; nothing strips it), so the + policy MUST either derive a version-free path from it or match in a way that + tolerates the prefix. The derivation MUST leave a request without a prefix + unchanged (a client may omit the version), strip at most one prefix, leave + `/_ping` and any other unversioned path alone, and yield no match for a path + carrying a `..` segment — the daemon cleans the path before routing while the + plugin authorizes the raw one. The documented input shape, and every probe + row, MUST use the values the plugin actually sends; a version-less probe + input proves nothing about a live daemon (issue #35). ## Design Principles Binding the Implementation @@ -447,8 +491,9 @@ deployment that does not accept them is deploying something else. sandbox container's bind-mounted config directory, plus the socket-mount precondition. - **Policy Reload** (`modules/policy-reload.md`) — host-side procedure to apply - a Rego policy change by bouncing the OPA plugin (no daemon restart), with the - pre-flight checks that keep a bad policy from being deployed. + a Rego policy change: an atomic file replacement (the plugin re-reads the + policy per request), with the pre-flight checks that keep a bad policy from + being deployed, and why a plugin bounce is fatal rather than cheap. ## Interfaces and Contracts @@ -474,8 +519,20 @@ for the `AuthZPlugin.AuthZReq` / `AuthZPlugin.AuthZRes` message schema. ### Rego input schema (contract consumed by the policy) -The plugin passes the Docker API request to the Rego evaluation in this shape -(documented by the plugin, with its additions marked): +The plugin passes the Docker API request to the Rego evaluation in this shape. +The values are the plugin's own, quoted from `main.go`'s `makeInput` — the +obvious-looking rewrite of two of them is what issue #35 was: + +```go +input := map[string]interface{}{ + "Headers": r.RequestHeaders, + "Path": r.RequestURI, // raw URI: version prefix + query + "PathPlain": u.Path, // raw path: version prefix, no query + "PathArr": strings.Split(u.Path, "/"), + "Query": u.Query(), + ... +} +``` ```json { @@ -483,8 +540,9 @@ The plugin passes the Docker API request to the Rego evaluation in this shape "AuthMethod": "TLS", "Method": "POST", "Path": "/v1.47/containers/create?name=backend-services-web-1", - "PathPlain": "/containers/create", + "PathPlain": "/v1.47/containers/create", "PathArr": ["", "v1.47", "containers", "create"], + "Query": { "name": ["backend-services-web-1"] }, "Headers": { "X-Sandbox-Agent": "true" }, "Body": { "Labels": { "com.docker.compose.project": "backend-services" }, @@ -499,20 +557,51 @@ The plugin passes the Docker API request to the Rego evaluation in this shape } ``` +**`PathPlain` carries the API version prefix.** The example above shows it +(`/v1.47/containers/create`), and a real plugin sends exactly that: nothing in +`main.go` strips the prefix, and `-skip-ping` only bypasses `HEAD /_ping`. A +policy that matches the raw field by equality therefore decides nothing on a +live daemon. That was issue #35 — the hardening closed the host-access holes +and, with them, every create, build, pull, delete and lifecycle grant — and it +failed closed, which is why it looked secure rather than broken. + +The policy derives the version-free path once (R-19) and every rule matches +that: + +```rego +path := p if { + p := regex.replace(object.get(input, "PathPlain", ""), "^/v[0-9]+(\\.[0-9]+)?", "") + not traversal(p) +} + +path_segments := split(path, "/") +``` + +The strip is anchored and single, so: `/v1.56/containers/create` → +`/containers/create`; `/containers/create` (a client that sends no version) → +unchanged; `/v1/containers/create` → `/containers/create`; `/_ping` → +unchanged; `/v1.56/v1.56/containers/create` → `/v1.56/containers/create`, which +is no grant. A path with a `..` segment yields no `path` at all, so no rule can +match it. + A create request carries no container name in its body: Docker takes it from the -`name` query parameter, which is why the policy reads the name of a container -create from the path and the name of a network or volume create from -`Body.Name` (those two endpoints take it in the body). The example above is the -compose-labelled create, the one case where a container create is granted. +`name` query parameter (which lands in `input.Query`), the name of a network or +volume create from `Body.Name`. The example above is the compose-labelled +create, the one case where a container create is granted. Fields used by the policy: - `input.User` — the TLS client certificate's subject common name; empty for unix-socket clients and for TCP clients with no certificate - `input.Method` — HTTP method (`GET`, `HEAD`, `POST`, `DELETE`) -- `input.PathPlain` — request path without the API version prefix or query; - the policy matches grants against it by **equality** (R-17) -- `input.PathArr` — the same path split into elements (a plugin addition); used - for whole-segment matching of names in the path +- `input.PathPlain` — the raw request path: **with** the API version prefix, + without the query string (`u.Path`). Read only to derive `path`; no rule + matches the raw field (R-19) +- `input.PathArr` — that path split on `/` (`["", "v1.47", "containers", + "create"]`). Not read by the policy: the derived `path_segments` is used for + whole-segment matching, so the version element cannot reach a match +- `input.Query` — the parsed query string, as a map of arrays (`{"name": + ["backend-services-web-1"]}`). Where a container create's name arrives; no + grant depends on it - `input.Headers` — request headers; the `P-9` header is the secondary identity signal, and header values are strings (`map[string]string`), never arrays - `input.Body` — the decoded request body, or `null` for requests without one @@ -831,16 +920,32 @@ Steps: ``` Output must be empty. If it is not, stop and fix the substitution. 3. Validate the policy against an engine **no newer than the plugin's** (P-10): - `opa check /tmp/agent.rego`. Without an `opa` binary, deploy and then verify - with the Phase 8 denial test instead. -4. Install it where the plugin's `-policy-file` argument points: - `install -D -m 644 /tmp/agent.rego P-7/agent.rego` -5. Reload so the plugin compiles the new file: + `opa check /tmp/agent.rego`, or the engine's own image when no binary is + installed — + `docker run --rm -v "$(pwd):/w:ro" -w /w openpolicyagent/opa:1.3.0 check agent.rego` + (1.3.0 is what `v0.10` embeds; never check with a newer engine than the + plugin's). The deploy script does this step itself. +4. Deploy it where the plugin's `-policy-file` argument points: `scripts/reload-opa-policy.sh /tmp/agent.rego` - The script runs the same leftover check, keeps the previous policy for - rollback, and bounces the plugin. **While the plugin is disabled every API - call is denied**, host users included — the window is short, but it is a - denial window, not an open one. + That path is P-7, and it does not have to be guessed — the installed plugin + reports it, since `docker plugin inspect` shows both the policy argument and + the mount carrying it (`-policy-file /opa/authz/agent.rego` with + `/etc/docker -> /opa` means `/etc/docker/authz/agent.rego`). The script finds + it the same way when `POLICY_DST`/`POLICY_DIR` are unset, and + `scripts/reload-opa-policy.sh --discover` prints it without changing + anything. + There is nothing to bounce: the plugin re-reads the policy **file on every + request**, so the change is live on the next API call. The script runs the + same leftover check, parses the policy under the plugin's engine, keeps the + previous policy at `P-7/agent.rego.previous` for rollback, and installs the + new file by writing a temporary file and renaming it over the target. Both + halves are load-bearing: + - **Never `docker plugin disable` / `rm` / `upgrade` a plugin the running + daemon references**: dockerd exits with `Error validating authorization + plugin ... not found` (measured; Q-4). Nothing in a policy update needs it. + - **Never copy or truncate the live file**: while it is absent the plugin + **fails open** (`OPA policy file ... does not exist, failing open and + allowing request`). The rename is what makes the replacement atomic. Verify: ``` @@ -949,13 +1054,20 @@ Each test names the requirements it covers. `...` stands for - **A-7** (covers R-8): a lifecycle action from the allowlist is allowed — `docker ... stop ` exits 0, and `docker ... exec ls` is denied (R-6). -- **A-8** (covers R-9): a policy update is a plugin bounce, not a daemon restart - — change the policy, run `scripts/reload-opa-policy.sh`, then re-run A-3 and - A-4: the read still works, the denial still denies, and the daemon's start time - is unchanged (`systemctl show --property=ExecMainStartTimestamp`). +- **A-8** (covers R-9, R-19): a policy update is a file deployment — not a + plugin bounce, not a daemon restart. Change the policy, run + `scripts/reload-opa-policy.sh`, then re-run A-3 and A-4: the read still + works, the denial still denies, the daemon's start time is unchanged + (`systemctl show --property=ExecMainStartTimestamp`), and + `docker plugin ls` shows the plugin's enabled state unchanged. Confirm which + policy is live from the plugin's own decision log: it logs `config_hash`, the + sha256 of the bytes it read, which must equal `sha256sum P-7/agent.rego` after + the next API call. Reversing the change is `scripts/reload-opa-policy.sh + --rollback`, and it must restore the previous decisions without touching the + plugin or the daemon either. - **A-9** (covers R-13): the deployed policy has no leftover placeholders — the `awk` check from Phase 6, step 2, over `P-7/agent.rego`, prints nothing. - Run it *before* the reload: a file that fails this check turns A-4 into a + Run it *before* deploying: a file that fails this check turns A-4 into a false pass. - **A-10** (covers R-3): the plugin was installed with its policy argument — `docker plugin inspect ` contains `policy-file` (or `config-file`), and @@ -977,14 +1089,23 @@ Each test names the requirements it covers. `...` stands for expects — `openssl x509 -in P-8/cert.pem -noout -subject` shows `CN=P-4`, its `extendedKeyUsage` is client authentication, and the server certificate's SANs cover P-1. -- **A-15** (covers R-5, R-6, R-7, R-8, R-11, R-12, R-17): the policy's decision - table — the probes in `skeleton/agent.rego.schema`, run with the plugin - image's own engine (P-10) against the deployed file, produce the expected - allow/deny for **every** row, including the exact-path rows (a smuggled body - on `/containers//attach` and `/exec` must be denied), the `POST /session` - grant, the testcontainers label, and both directions of the project-scoped - create checks. This is the cheapest test to run and the only one that - exercises policy branches a live host cannot easily reach. +- **A-15** (covers R-5, R-6, R-7, R-8, R-11, R-12, R-17, R-19): the policy's + decision table — the probes in `skeleton/agent.rego.schema`, run with the + plugin image's own engine (P-10) against the deployed file, produce the + expected allow/deny for **every** row, including the exact-path rows (a + smuggled body on `/containers//attach` and `/exec` must be denied), the + `POST /session` grant, the testcontainers label, and both directions of the + project-scoped create checks. + **The rows must be fed the plugin's real input shape** (`Path`, `PathPlain` + and `PathArr` carrying the version prefix, `Query` parsed) — a table cannot + catch a disagreement between its own model of the input and the plugin's + (`main.go`'s `makeInput`) unless it uses the plugin's values. Run the table + **twice** where the shape is in question: once with a version prefix and once + without, and require identical decisions. + **This test alone is not enough**, and #35 is the proof: with a wrong input + shape the whole table passed while every create on a live host was denied. The + live tests (A-11, A-16) are the ones that catch that class; run at least one + of them against a real daemon before trusting a policy change. - **A-16** (covers R-15, R-16): the host-access gate holds for a project-labelled client — from the sandbox, each of these is denied by the plugin, and the denial is not a mistake of syntax but the R-15/R-16 decision: @@ -1147,7 +1268,10 @@ Decisions: - **Reload semantics were wrong in one direction.** The package said a policy reload was unblocked; in fact a plugin bounce is a *denial* window (the daemon fails closed while the plugin is down), and a plugin installed - without `opa-args` fails *open*. Both are now stated where they apply, and + without `opa-args` fails *open*. **Corrected 2026-09-17**: the bounce is + worse than a denial window — with the plugin referenced by a running daemon + it is fatal to the daemon, and it was never needed in the first place + (Q-4, below). Both are now stated where they apply, and the install step passes the policy argument explicitly. - **Two failure-behavior claims were corrected**: the daemon fails closed on a plugin error (documented), and the plugin allows everything when it has no @@ -1248,6 +1372,71 @@ Decisions: deletion of the per-package `SCHEMATIC.md.schema` is in the base rather than a conflict here. +- 2026-09-17 (later same day) — **The path contract was wrong: the hardening above was dead on a live daemon** (#35). Everything below is measured, not read. + - **The plugin's `PathPlain` carries the API version prefix.** `main.go`'s + `makeInput` sets `"PathPlain": u.Path` and `"PathArr": + strings.Split(u.Path, "/")` (line 257) from the raw request URL; nothing + strips a version (`-skip-ping` only bypasses `HEAD /_ping`). The decision + log of the running plugin shows it plainly: `"Path": + "/v1.56/containers/json"`, `"PathPlain": "/v1.56/containers/json"`, + `"PathArr": ["","v1.56","containers","json"]`. + - **So the equality grants R-17 introduced matched nothing** — and neither did + the `startswith` rules behind lifecycle actions, container deletion and + path-named network/volume calls. On a live daemon the sandbox could not + create a container, volume or network for its own project, and could not + start, stop or delete one. The documented input shape in this file showed a + version-less `PathPlain`, and the 0.4.0 probe table was built from that + shape, so the table agreed with a fiction. + - **It was invisible because it failed closed.** A sandbox that cannot create + anything looks secure rather than broken. Only a test that uses the real + plugin — or its real field values — can catch this class; the table alone + did not, and A-15 now states that limit. + - **The fix derives the version-free path inside the policy** (R-19): `path` + is `PathPlain` with one optional `/v[.]` prefix removed and + with no `..` segment, and `path_segments` is `split(path, "/")`, so + `PathArr` is no longer read at all. Deriving in the policy rather than + rebuilding from `PathArr` was chosen because rebuilding mishandles a path + with no version (`/_ping` would lose a real segment) and needs the same + guard anyway, while an anchored regex leaves an unversioned path untouched — + a client that omits the version keeps working. The `..` guard is new: the + daemon cleans the path before routing while the plugin authorizes the raw + one, so no rule should be reachable through a traversal. + - **Evidence, in the real input shape** (`Path` and `PathPlain` the raw + versioned path, `PathArr` split from that, `Query` parsed) — 78 rows on + three engines: the 0.4.0 policy decided wrongly on 26 of them, every one a + legitimate request **denied** (creates, network and volume creates, deletes, + lifecycle actions, build, pull, `/session`); the fixed policy decides all 78 + as specified on OPA v0.60.0, v1.3.0 and v1.7.1. The same table run with the + version prefix removed, and with `v1.43`, gives identical decisions — the + property R-19 states. + - **Version 0.5.0 (minor)**: R-19 is a *new* requirement (path matching must + be version-independent), not a re-wording of an existing one; the defect + alone would have been a patch. + - **Q-4 answered — and the reload procedure was wrong twice.** Disabling a + plugin the running daemon references is FATAL: `level=fatal msg="Error + validating authorization plugin" error="plugin \"\" not found"`, and + dockerd exits (measured on Docker 29.6.1, snap install). In that state + `docker plugin enable` cannot help, because it needs a running daemon — the + recovery is to remove the reference from the live configuration file (P-13) + first. But the bounce was never *needed*: `evaluatePolicyFile` reads the + policy **file on every request**, so the deployed file *is* the live policy. + The procedure is now a file replacement — install to a temporary name, then + `mv -f` over the target — because a *missing* policy file is the plugin's + one fail-**open** path (`OPA policy file %s does not exist, failing open and + allowing request`), which `install -D` over the live path could create. + `scripts/reload-opa-policy.sh` no longer touches plugin state, and verifies + a deployment through the decision log's `config_hash` instead. + - **Q-5 answered — yes**: a snap-installed daemon does bind-mount host `P-6` + into the managed plugin at `/opa`; verified by the operator on the snap host + this package was tested against. + - **Deployment note from the tested host**: the plugin registers as + `opa-docker-authz:latest` (installed without `--alias`), which is the name + the daemon's `authorization-plugins` entry must carry, and the name that + appears in denial messages. P-14 is a parameter for exactly this reason. + - **Residual**: the live end-to-end pass (A-11, A-16) belongs to the operator + against a real daemon; this package's evidence is the table above plus the + measured plugin behaviour it rests on. + Open questions: - **Q-1** *(answered 2026-09-16; re-verified 2026-09-17)*: @@ -1276,14 +1465,13 @@ Open questions: `inferred:` and untested here. Decide by testing on a maintenance window with a restart, not by assumption — until then, Phase 3's ordering (install first, reference second) is the safe procedure. -- **Q-5** *(new 2026-09-17, raised in review)*: on a snap-installed daemon, - whether the confined daemon can bind-mount host `P-6` into the managed plugin - at `/opa` at all. The plugin path (`/opa/authz/agent.rego`) and the install - argument are documented and consistent on the distribution package; the snap's - confinement is `inferred:` and untested here, and if it cannot, the policy - file must live somewhere the daemon can already see. Decide by installing the - plugin on a snap host and reading `docker plugin inspect ` — Phase 3 - succeeds either way; it is the policy load that would fail. +- **Q-5** *(answered 2026-09-17)*: **yes** — a snap-installed daemon does + bind-mount host `P-6` into the managed plugin at `/opa`, verified by the + operator on the snap host this package was tested against (the daemon's + `/etc/docker` is a snap layout pointing at `$SNAP_DATA/etc/docker`, and the + plugin's mount source resolves there). The install argument and the policy + path (`/opa/authz/agent.rego`) are therefore correct as documented, and no + second location for the policy file is needed. diff --git a/schematics/authorize-docker-requests/modules/opa-policy.md b/schematics/authorize-docker-requests/modules/opa-policy.md index bc4a82d..db879bc 100644 --- a/schematics/authorize-docker-requests/modules/opa-policy.md +++ b/schematics/authorize-docker-requests/modules/opa-policy.md @@ -6,7 +6,7 @@ authorization plugin on every daemon request. ## Purpose -Implements the authorization rules defined in R-1 through R-18. The policy +Implements the authorization rules defined in R-1 through R-19. The policy distinguishes sandbox clients (by TLS certificate CN or HTTP header) from local host users, grants the sandbox read-only access by default, and then selectively permits compose project operations, image builds, pulls, and a closed lifecycle @@ -32,6 +32,11 @@ container, in its project" are different grants: `/exec` must not satisfy the create rule. - **R-18** — network and volume deletes are scoped by the project as a whole path segment, the same way creation is scoped by name. +- **R-19** — path matching is version-independent. `path` is `PathPlain` with + one optional `/v[.]` prefix removed and no `..` segment; every + rule matches that, so the same policy decides the same on a live daemon (which + sends `/v1.56/containers/create`) and for a client that omits the version + (which sends `/containers/create`). ## Inputs @@ -44,11 +49,18 @@ before evaluation. Fields the policy uses: - `input.AuthMethod` — string. `TLS` when the client authenticated with a certificate. - `input.Method` — string. HTTP method: `GET`, `HEAD`, `POST`, `DELETE`. -- `input.PathPlain` — string. Request path without the query string, e.g. - `/containers/json`, `/images/create`, `/build`. Grants are matched against it - with `==` (R-17). -- `input.PathArr` — array. `PathPlain` split into path elements. Used to match a - resource named in the path as a whole segment rather than as a substring. +- `input.PathPlain` — string. The **raw request path**: API version prefix + included, query string excluded (`u.Path`), e.g. `/v1.56/containers/json`. + Nothing strips the version (issue #35), so the policy derives `path` from it + and matches that with `==` (R-17, R-19) — the raw field is read for nothing + else. +- `input.PathArr` — array. `PathPlain` split into path elements, so its second + element is the version. Not read by the policy: `path_segments` is the derived + path split the same way, which is what matches a resource named in the path as + a whole segment rather than as a substring. +- `input.Query` — object. The parsed query string as a map of arrays. A + container create's name arrives here (Docker takes it from the `name` query + parameter, not the body); no grant depends on it. - `input.Headers` — object. Header names to header values, as plain strings (`map[string]string` in Docker's own message type, so a value is never an array). The header named by `P-9` with the value `true` is the secondary @@ -94,8 +106,8 @@ so the plugin release decides the language version: | Plugin image | Embedded OPA | Result | |--------------|--------------|--------| -| `ghcr.io/open-policy-agent/opa-docker-authz:v0.10` | **v1.3.0** | loads; every one of the 71 probe rows decides as specified | -| `openpolicyagent/opa-docker-authz-v2:0.9` | v0.60.0 | loads; identical decisions on all 71 probe rows | +| `ghcr.io/open-policy-agent/opa-docker-authz:v0.10` | **v1.3.0** | loads; every one of the 78 probe rows decides as specified | +| `openpolicyagent/opa-docker-authz-v2:0.9` | v0.60.0 | loads; identical decisions on all 78 probe rows | | `openpolicyagent/opa-docker-authz-v2:0.8` | v0.30.0 | does **not** load — `import rego.v1` is rejected | The embedded versions are read from each release's own `go.mod` at its tag @@ -138,6 +150,12 @@ limitations): of those has a form that is allowed (no `DriverOpts`, an explicit network, a name); a deployment that needs the refused form must extend the policy deliberately, and the probe table must grow a row with it. +- **The table is not the daemon.** A probe row decides about the input it + carries; if that input is not the plugin's, the table agrees with a fiction. + Until 0.5.0 every row carried a version-less `PathPlain` that the plugin never + sends, so the table passed while live creates were denied (issue #35). Rows + now use the plugin's values (`main.go`'s `makeInput`), and the live tests are + what prove the values are the plugin's. - **Host port publishing is not part of the gate.** A project container may publish a host port (`ports:`), which does not read the host filesystem but can occupy a free port and answer for it. Closing that is the daemon diff --git a/schematics/authorize-docker-requests/modules/policy-reload.md b/schematics/authorize-docker-requests/modules/policy-reload.md index 9457c9f..3ed662c 100644 --- a/schematics/authorize-docker-requests/modules/policy-reload.md +++ b/schematics/authorize-docker-requests/modules/policy-reload.md @@ -1,53 +1,78 @@ # Module: Policy Reload -Host-side procedure to apply a Rego policy change without restarting the Docker -daemon. The daemon restart is the expensive move (it stops containers); the -plugin is the cheap one. +Host-side procedure to apply a Rego policy change. On this package's default +install there is nothing to reload: **the plugin re-reads the policy file on +every request**, so the change is live on the next API call. ## Purpose -Policy updates should not require a daemon restart. What a policy update *does* -cost depends on how the plugin was installed, and the difference matters -operationally: +A policy update must not cost a daemon restart, and on the file path it does not +even cost a plugin bounce. `main.go` (`evaluatePolicyFile`) reads the policy +file, compiles it, and evaluates it *inside the handling of a single request*: -| Path | How it reloads | What the window looks like | -|------|----------------|---------------------------| +```go +bs, err := os.ReadFile(p.policyFile) +... +eval := rego.New(rego.Query(p.allowPath), rego.Input(input), + rego.Module(p.policyFile, string(bs))) +``` + +and it logs a sha256 of the bytes it read as the decision log's `config_hash`. +So the deployed file *is* the live policy, per request. Two properties of that +code decide the whole procedure: + +| Deployment path | How a change takes effect | What the window looks like | +|---|---|---| +| `-policy-file` (**this package's install**) | The file is read per request: replace it and the next API call uses the new policy | No window, **provided the file is replaced atomically** — see below | | `-config-file` with a bundle service | The plugin long-polls its bundle endpoint; a new bundle applies without touching the plugin process | No window: the old policy is in force until the new bundle is fetched | -| Managed plugin, `-policy-file` | Disable and re-enable the plugin so it re-reads the file | **Denial window**: while the plugin is unavailable, the daemon's authorization middleware fails closed and every API call is denied, host users included | -| Legacy plugin container | Restart the container | Same denial window | +| `docker plugin disable` → `enable` | Not a reload path at all: it stops the plugin the daemon is using | **Fatal** — see the next section | -The plugin's own documentation recommends the bundle form for exactly this -reason (plus decision logging). The file form is simpler to deploy and is what -this package's default install uses. +**What a policy change is not:** -**What a reload is not:** removing the plugin reference from the live daemon -configuration file (P-13) and -sending SIGHUP is not a reload, it is a return to an unrestricted daemon — every -request is allowed while the entry is absent. Use it only as the deliberate -rollback, never as a step in a policy update. +- **It is not a plugin bounce.** Disabling, removing, or upgrading a plugin that + the running daemon references makes dockerd treat its own configuration as + invalid and **exit**: `level=fatal msg="Error validating authorization + plugin" error="plugin \"\" not found"` (measured on Docker 29.6.1, snap + install; Q-4 in SCHEMATIC.md's decisions). Before this was measured, the + script this package shipped did exactly that. +- **It is not removing the plugin reference.** Deleting `P-14` from + `authorization-plugins` and sending SIGHUP leaves an unrestricted daemon: + every request is allowed while the entry is absent. That is the deliberate + rollback of the whole capability, never a step in a policy update. ## Inputs - Parameters P-6, P-7, P-13 from SCHEMATIC.md. - The updated policy source (the implementer's working copy, e.g. in a git repository), with every placeholder substituted. -- The plugin installed and enabled (Phase 3). +- The plugin installed and enabled (Phase 3). Its `-policy-file` argument and + mount are read to find P-7 — `docker plugin inspect ` names both, so the + path is derivable rather than guessed, and + `scripts/reload-opa-policy.sh --discover` prints it read-only. ## Outputs -- `P-7/agent.rego` — the deployed policy, copied from the substituted source. -- Plugin state: bounces through disable → enable on the file path; unchanged on - the bundle path. +- `P-7/agent.rego` — the deployed policy, installed by atomic rename. +- `P-7/agent.rego.previous` — the policy it replaced, for rollback. +- Plugin state: **unchanged**. Nothing in this procedure disables, restarts, or + reinstalls the plugin, and no daemon restart is involved. -## Pre-flight checks (before touching the running plugin) +## Pre-flight checks (before touching the deployed file) -1. **No placeholders left**: `grep -nE 'SANDBOX_USERNAME|AUTH_HEADER_NAME|PROJECT_NAME|PROJECT_DIR_PATH|BUILDKIT_PREFIX|TESTCONTAINERS_LABEL' ` must print nothing. A leftover token would be deployed as a literal and silently turn the policy into "allow everything". (`BUILDKIT_PREFIX` is retired — R-17 removed the BuildKit carve-out — and is kept in this pattern on purpose: the check is a superset of the template's tokens, so a source copied from the pre-0.4.0 template is still caught.) -2. **It parses**: `opa check ` with an `opa` binary at or below the - plugin's engine version (see `skeleton/agent.rego.schema`). +1. **No placeholders left**: `grep -nE 'SANDBOX_USERNAME|AUTH_HEADER_NAME|PROJECT_NAME|PROJECT_DIR_PATH|BUILDKIT_PREFIX|TESTCONTAINERS_LABEL' ` must print nothing. A leftover token would be deployed as a literal and silently turn the policy into "allow everything". (`BUILDKIT_PREFIX` is retired — R-17 removed the BuildKit carve-out — and is kept in this pattern on purpose: the check is a superset of the template's tokens, so a source copied from an older template is still caught.) +2. **It parses**, under an engine **no newer than the plugin's** (P-10): + `opa check `, or the engine's own image when no binary is installed — + `docker run --rm -v "$(pwd):/w:ro" -w /w openpolicyagent/opa:1.3.0 check `. + A newer engine accepts syntax the plugin's engine rejects. 3. **It decides correctly**: the `opa eval` probes in - `skeleton/agent.rego.schema` still produce the expected allow/deny results. -4. **A copy of the currently deployed policy is kept**, so the change can be - reverted with the same procedure. + `skeleton/agent.rego.schema` still produce the expected allow/deny results — + **with the plugin's real input shape**, which is the part this package got + wrong once: `PathPlain` carries the API version prefix (`"PathPlain": u.Path` + in `main.go`), so a probe table written with a version-less `PathPlain` + proves nothing about a live daemon (issue #35). Every path in the table is + `/v1./…`, and the runner records the version it used. +4. **A copy of the currently deployed policy is kept** before the replacement, + so the change can be reverted with the same procedure. ## Dependencies @@ -56,27 +81,55 @@ rollback, never as a step in a policy update. ## Failure Behavior -- **Policy syntax error**: the plugin does not serve a decision, so the daemon - fails closed and all API calls are denied until a valid policy is deployed. - This is an outage, not a security hole. Recovery: restore the previous file - and re-apply the reload. -- **Plugin already disabled**: the disable step fails or is skipped; the script - in `scripts/reload-opa-policy.sh` checks the state before acting. -- **Plugin not found**: it was never installed. Run Phase 3 first — and note - that a plugin installed without `opa-args` answers "allow" to everything, so - "install it and move on" is not a valid recovery. -- **Source file not found**: the copy fails. Pass an explicit path. +- **Policy syntax error**: the plugin cannot compile it, so it returns an error + and the daemon fails closed — an outage, not a security hole. Recovery: + `--rollback` (restores `agent.rego.previous`) or re-deploy a valid file. +- **The policy file is missing when a request arrives**: the plugin **fails + open** (`OPA policy file %s does not exist, failing open and allowing + request`). This is the one open direction in this procedure, and it is why + the deployed file is replaced by `install` to a temporary name followed by + `mv -f` — one atomic rename — and never by copying or truncating the live + path. +- **A partially written policy file**: it does not compile, so the request is + denied and logged; the atomic rename makes this unreachable in the normal + flow. +- **The plugin is missing, disabled, or was removed from the daemon's + configuration**: dockerd will not complete a start or a reload — it exits + with `Error validating authorization plugin`. If that state exists while the + daemon is configured to reference the plugin, the daemon crash-loops, and + `docker plugin enable` cannot help because it needs a running daemon: + recovery is to remove the reference from the live daemon configuration file + (P-13), start the daemon (unrestricted for the moment), `docker plugin + enable `, then put the reference back and apply it with SIGHUP. This is + why nothing in this procedure touches plugin state. +- **Source file not found**: nothing is replaced. Pass an explicit path. - **Bundle path unavailable**: with `-config-file`, the plugin keeps serving the last bundle it fetched; a decision-log or plugin-log line reports the fetch failure. The daemon is not disrupted, and the policy is stale rather than absent — state the staleness in the change record. +## Verification (not just "the file looks right") + +0. `scripts/reload-opa-policy.sh --discover` prints the host path the plugin + actually reads, derived from its `-policy-file` argument and the mount that + carries it. Compare that with P-7 before deploying: a policy written to a + directory the plugin does not mount is installed perfectly and read by + nobody. +1. `sha256sum P-7/agent.rego`, then read the plugin's decision log for the next + request and compare with its `config_hash` — the plugin logs the hash of the + bytes it actually evaluated, so this is the difference between "the file on + disk changed" and "the live policy changed". +2. A decision that the change was about: one that must be allowed and one that + must be denied, over the TLS listener (Phase 8's tests). + ## Idempotency Notes -- Copying the policy file is idempotent (it always overwrites with the source). -- Disable-then-enable is idempotent only if the plugin is currently enabled. +- Installing the file is idempotent: the same source always produces the same + deployed bytes, and `agent.rego.previous` is a copy of what was replaced. - Substitution is idempotent, and the no-placeholder check makes a second run against an already-deployed file a no-op rather than a corruption. +- Re-running the procedure never leaves the plugin in a different state from + the one it started in. ## Removal Notes diff --git a/schematics/authorize-docker-requests/scripts/reload-opa-policy.sh b/schematics/authorize-docker-requests/scripts/reload-opa-policy.sh index 618e10e..16fd914 100755 --- a/schematics/authorize-docker-requests/scripts/reload-opa-policy.sh +++ b/schematics/authorize-docker-requests/scripts/reload-opa-policy.sh @@ -1,50 +1,129 @@ #!/usr/bin/env bash -# ─── Reload the OPA policy without restarting the Docker daemon ── +# ─── Deploy a Rego policy change without restarting dockerd ────── # -# Run this on the HOST (not inside the sandbox) after changing the Rego -# policy. It validates the substituted policy, installs it where the plugin -# reads it, and bounces the plugin so it is compiled again. +# The plugin re-reads the policy FILE ON EVERY REQUEST, so a policy change +# takes effect on the next API call and there is nothing to bounce. main.go +# (evaluatePolicyFile) does `os.ReadFile(p.policyFile)` inside the evaluation +# of every request, then logs a sha256 of the bytes it read as the decision +# log's `config_hash`. Two consequences decide this whole procedure: # -# Idempotent: safe to re-run. The bounce is guarded by a state check. -# -# What the bounce costs: while the plugin is disabled the daemon's -# authorization middleware fails closed, so EVERY API call is denied -# (host users included) until it is enabled again. That window is short, but -# it is a denial window, not an open one. If a plugin-free policy change is -# required, install the plugin with `-config-file` and a bundle service -# instead — see modules/policy-reload.md. -# -# Prerequisites: -# - the authorization plugin installed (with opa-args) and enabled -# - the policy source fully substituted: no PLACEHOLDER tokens left +# 1. `docker plugin disable` / `enable` / `rm` / `upgrade` is NOT part of a +# policy change, and is FATAL while the daemon references the plugin: a +# missing or disabled plugin makes dockerd exit — +# level=fatal msg="Error validating authorization plugin" +# error="plugin \"opa-docker-authz\" not found" +# (measured on Docker 29.6.1, snap install). Whether that costs only the +# API or every container depends on the unit's restart policy and +# live-restore — do not find out on purpose. +# 2. The policy file must never be *absent* on a request: a missing file is +# the plugin's one fail-OPEN path, and it says so in its log — +# OPA policy file %s does not exist, failing open and allowing request +# So the file is replaced by writing a temporary file and renaming it over +# the target (one atomic rename on the same filesystem), never by copying +# or truncating the live path. # # Usage: -# sudo ./reload-opa-policy.sh # uses ./skeleton/agent.rego -# sudo ./reload-opa-policy.sh /path/to/agent.rego +# sudo ./reload-opa-policy.sh [substituted-agent.rego] # default ./agent.rego +# sudo ./reload-opa-policy.sh --rollback # restore .previous +# ./reload-opa-policy.sh --discover # print the policy path +# +# The policy path is read from the plugin itself, so it does not have to be +# guessed: `docker plugin inspect` shows both the argument the plugin runs with +# (`-policy-file /opa/authz/agent.rego`) and the mount that carries it +# (`/etc/docker -> /opa`), and together they are the host path to write (this is +# P-7). `--discover` reports it and exits. # # Environment overrides: -# POLICY_DIR where the plugin reads the policy (default /etc/docker/authz) +# POLICY_DST host path of the policy file to replace (wins over POLICY_DIR) +# POLICY_DIR host directory the plugin reads the policy from # PLUGIN installed plugin name (default opa-docker-authz) -# OPA_BIN path to an opa binary for `opa check` (optional) +# OPA_BIN opa binary for `opa check` (optional; used if set) +# OPA_IMAGE image for `opa check` instead (default openpolicyagent/opa:1.3.0) +# SKIP_CHECK set to 1 to skip `opa check` (not recommended) +# +# Prerequisites: the authorization plugin installed WITH its policy argument, +# and the policy source fully substituted (no PLACEHOLDER tokens left). # ═══════════════════════════════════════════════════════════════════ set -euo pipefail -POLICY_DIR="${POLICY_DIR:-/etc/docker/authz}" -POLICY_SRC="${1:-./skeleton/agent.rego}" -POLICY_DST="${POLICY_DIR}/agent.rego" PLUGIN="${PLUGIN:-opa-docker-authz}" OPA_BIN="${OPA_BIN:-}" +OPA_IMAGE="${OPA_IMAGE:-openpolicyagent/opa:1.3.0}" +SKIP_CHECK="${SKIP_CHECK:-0}" +POLICY_DIR="${POLICY_DIR:-}" +POLICY_DST="${POLICY_DST:-}" PLACEHOLDERS='SANDBOX_USERNAME|AUTH_HEADER_NAME|PROJECT_NAME|PROJECT_DIR_PATH|BUILDKIT_PREFIX|TESTCONTAINERS_LABEL' -# ─── Step 1: validate the source policy ───────────────────────── +# ─── Where the plugin reads its policy ────────────────────────── +# Echoes the host path of the file the plugin's -policy-file argument names, +# by mapping that in-plugin path back through the plugin's own bind mount. +discover_policy_file() { + local inpath src dest rest + inpath="$(docker plugin inspect "$PLUGIN" \ + --format '{{range .Settings.Args}}{{println .}}{{end}}' 2>/dev/null \ + | awk '/^-policy-file$/{getline; print; exit}')" || true + if [ -z "$inpath" ]; then + inpath="$(docker plugin inspect "$PLUGIN" \ + --format '{{range .Settings.Args}}{{println .}}{{end}}' 2>/dev/null \ + | sed -n 's/^-policy-file=//p' | head -1)" + fi + [ -n "$inpath" ] || return 1 + while IFS='|' read -r src dest; do + [ -n "$src" ] && [ -n "$dest" ] || continue + case "$inpath" in + "$dest"/*) rest="${inpath#"$dest"}"; printf '%s%s\n' "${src%/}" "$rest"; return 0 ;; + "$dest") printf '%s\n' "$src"; return 0 ;; + esac + done < <(docker plugin inspect "$PLUGIN" \ + --format '{{range .Settings.Mounts}}{{.Source}}|{{.Destination}}{{println}}{{end}}' 2>/dev/null) + return 1 +} + +if [ "${1:-}" = "--discover" ]; then + if found="$(discover_policy_file)"; then + echo "$found" + exit 0 + fi + echo "ERROR: no policy path could be read from plugin ${PLUGIN}." >&2 + echo " Is it installed (docker plugin ls) and does it run with -policy-file?" >&2 + exit 1 +fi + +if [ -n "$POLICY_DST" ]; then + POLICY_DIR="${POLICY_DIR:-$(dirname "$POLICY_DST")}" +elif [ -n "$POLICY_DIR" ]; then + POLICY_DST="${POLICY_DIR}/agent.rego" +elif POLICY_DST="$(discover_policy_file)"; then + POLICY_DIR="$(dirname "$POLICY_DST")" + echo "==> policy path discovered from plugin ${PLUGIN}: ${POLICY_DST}" +else + POLICY_DIR="/etc/docker/authz" + POLICY_DST="${POLICY_DIR}/agent.rego" + echo "==> WARNING: no policy path could be read from plugin ${PLUGIN};" + echo " falling back to ${POLICY_DST} (override with POLICY_DST=)." +fi +POLICY_DIR="${POLICY_DIR%/}" +POLICY_PREV="${POLICY_DST}.previous" + +# ─── Rollback path ────────────────────────────────────────────── +ROLLBACK=0 +if [ "${1:-}" = "--rollback" ]; then + ROLLBACK=1 + POLICY_SRC="$POLICY_PREV" +else + POLICY_SRC="${1:-./agent.rego}" +fi + if [ ! -f "$POLICY_SRC" ]; then echo "ERROR: policy source not found: $POLICY_SRC" >&2 - echo " Pass the path as an argument, or run from the schematic root." >&2 + echo " Pass the substituted policy as an argument (--rollback uses" >&2 + echo " ${POLICY_PREV})." >&2 exit 1 fi +# ─── Step 1: validate the source policy ───────────────────────── leftover="$(awk -v pat="$PLACEHOLDERS" \ '!/^[[:space:]]*#/ && $0 ~ pat { printf "%s:%d: %s\n", FILENAME, FNR, $0 }' \ "$POLICY_SRC")" @@ -59,52 +138,63 @@ if [ -n "$leftover" ]; then fi echo "==> Step 1: policy has no placeholder tokens" -if [ -n "$OPA_BIN" ]; then - "$OPA_BIN" check "$POLICY_SRC" - echo " passed opa check: $OPA_BIN" +if [ "$SKIP_CHECK" != "1" ]; then + # Check with the engine the plugin embeds (P-10), never a newer one. + if [ -n "$OPA_BIN" ]; then + "$OPA_BIN" check "$POLICY_SRC" + echo " passed opa check: ${OPA_BIN}" + else + src_dir="$(cd "$(dirname "$POLICY_SRC")" && pwd)" + base="$(basename "$POLICY_SRC")" + docker run --rm -v "${src_dir}:/w:ro" -w /w "$OPA_IMAGE" check "$base" + echo " passed opa check under ${OPA_IMAGE} (the engine v0.10 embeds)" + fi +else + echo " WARNING: opa check skipped (SKIP_CHECK=1)" fi -# What is being replaced, kept for rollback. -if [ -f "$POLICY_DST" ]; then - backup="${POLICY_DST}.previous" - cp -p "$POLICY_DST" "$backup" - echo " previous policy kept at ${backup}" +# ─── Step 2: keep the current policy, then replace it atomically ─ +if [ "$ROLLBACK" = "1" ]; then + echo "==> Step 2: rolling back to ${POLICY_PREV}" + [ -f "$POLICY_PREV" ] || { echo "ERROR: ${POLICY_PREV} not found" >&2; exit 1; } +else + if [ -f "$POLICY_DST" ]; then + cp -p "$POLICY_DST" "$POLICY_PREV" + echo "==> Step 2: previous policy kept at ${POLICY_PREV}" + echo " roll back with: $0 --rollback" + else + echo "==> Step 2: no policy deployed yet; nothing to keep" + fi fi -install -D -m 644 "$POLICY_SRC" "$POLICY_DST" -echo " wrote ${POLICY_DST}" +# temp file + rename: the plugin must never see a missing or partial policy. +tmp="${POLICY_DST}.tmp.$$" +trap 'rm -f "$tmp"' EXIT +install -D -m 644 "$POLICY_SRC" "$tmp" +mv -f "$tmp" "$POLICY_DST" +trap - EXIT +echo " deployed atomically: ${POLICY_DST}" echo "" -# ─── Step 2: bounce the plugin so the policy is compiled ──────── -echo "==> Step 2: bounce ${PLUGIN}" - -if docker plugin ls --format '{{.Name}}|{{.Enabled}}' 2>/dev/null | grep -q "^${PLUGIN}|true$"; then - echo " disabling (API calls are denied while it is down)..." - docker plugin disable "$PLUGIN" - echo " re-enabling (authorization resumes)..." - docker plugin enable "$PLUGIN" - echo " plugin re-enabled." -elif docker plugin ls --format '{{.Name}}' 2>/dev/null | grep -q "^${PLUGIN}$"; then - echo " plugin exists but is disabled — enabling directly..." - docker plugin enable "$PLUGIN" -else - echo "ERROR: plugin ${PLUGIN} is not installed." >&2 - echo " Install it WITH its policy argument, or every request is allowed:" >&2 - echo " docker plugin install --grant-all-permissions --alias ${PLUGIN} \\" >&2 - echo " ghcr.io/open-policy-agent/opa-docker-authz:v0.10 \\" >&2 - echo " opa-args=\"-policy-file /opa/authz/agent.rego\"" >&2 - echo " (the host path /etc/docker is mounted at /opa inside the plugin)" >&2 - exit 1 -fi +# ─── Step 3: verify a decision, not just the file ─────────────── +new_hash="$(sha256sum "$POLICY_DST" | cut -d' ' -f1)" +echo "==> Step 3: verify" +echo " policy sha256: ${new_hash}" +echo " The plugin logs a sha256 of the bytes it read as the decision log's" +echo " \`config_hash\` on every request, so this proves which policy is live:" +echo " journalctl -u docker -n 200 | grep -o '\"config_hash\":\"[0-9a-f]*\"' | tail -1" +echo " (a snap install logs where the unit logs; use 'docker info' to confirm" +echo " the daemon, then that unit's journal). It must print the hash above" +echo " after the next API call — no plugin bounce, no daemon restart." echo "" - -# ─── Step 3: verify ───────────────────────────────────────────── -echo "==> Step 3: plugin status" -docker plugin ls --format '{{.Name}} enabled={{.Enabled}}' | grep "^${PLUGIN}" || true +echo " Then check the decisions the change was about, over the TLS listener:" +echo " docker --tlsverify -H tcp://: ... ps # allowed" +echo " docker --tlsverify -H tcp://: ... run --rm alpine true # denied" echo "" -echo " Now verify a decision, not just the plugin state:" -echo " docker --tlsverify -H tcp://: ... ps # allowed" -echo " docker --tlsverify -H tcp://: ... run --rm alpine echo hi # denied" -echo " If the second command succeeds, the policy is not in force — check for" -echo " leftover placeholders and that the plugin was installed with opa-args." +echo " If a request that should have been denied succeeds, the policy is not" +echo " in force: check for leftover placeholders, and that the plugin was" +echo " installed with its opa-args (a plugin without a policy allows all)." echo "" +echo "==> Plugin state (for the record — this script never changes it)" +docker plugin ls --format '{{.Name}} enabled={{.Enabled}}' 2>/dev/null | grep "^${PLUGIN}" \ + || echo " WARNING: plugin ${PLUGIN} not listed; it must exist and be enabled." diff --git a/schematics/authorize-docker-requests/skeleton/agent.rego b/schematics/authorize-docker-requests/skeleton/agent.rego index 4867fe2..63c3eb1 100644 --- a/schematics/authorize-docker-requests/skeleton/agent.rego +++ b/schematics/authorize-docker-requests/skeleton/agent.rego @@ -35,6 +35,11 @@ package docker.authz # request in that project's name that would reach the host (R-15, R-16). Every # create path passes `safe_container_config`; see the probe table in # agent.rego.schema for the requests this is asserted against. +# +# Paths are matched against `path`, not against the plugin's raw PathPlain +# field: the plugin sets PathPlain to the *raw request path*, API version +# prefix and all ("PathPlain": u.Path, main.go), so a policy keyed on the raw +# field decides nothing on a live daemon. See the derivation below. import rego.v1 @@ -63,6 +68,38 @@ allow if { # ─── Sandbox default ───────────────────────────────────────────── default allow_sandbox := false +# ─── Path handling ─────────────────────────────────────────────── +# The plugin builds the input from the raw request URL: `"PathPlain": u.Path` +# and `"PathArr": strings.Split(u.Path, "/")` (main.go, makeInput). Nothing +# strips the API version prefix — `-skip-ping` only bypasses `HEAD /_ping` — +# so on a live daemon PathPlain is `/v1.56/containers/create`, not +# `/containers/create`, and PathArr is `["", "v1.56", "containers", "create"]`. +# +# Every rule below therefore matches `path`: PathPlain with one optional +# version segment removed. The strip is anchored and single, and a request +# that carries no version is left alone, so all of these behave as the API +# says they should: +# +# /v1.56/containers/create → /containers/create +# /containers/create → /containers/create (client sends no version) +# /v1/containers/create → /containers/create (major only) +# /_ping → /_ping (nothing to strip) +# /v1.56/v1.56/x → /v1.56/x (one strip, still no grant) +# +# `path_segments` is the same path split on "/", so a resource named in the +# path is matched as a whole segment and neither the version segment nor a +# query string can satisfy a match. +# +# A path with a `..` segment yields no `path` at all, so no rule can match it: +# the daemon cleans the path before routing, and the plugin authorizes the raw +# one, so nothing in this policy should be reachable through a traversal. +path := p if { + p := regex.replace(object.get(input, "PathPlain", ""), "^/v[0-9]+(\\.[0-9]+)?", "") + not traversal(p) +} + +path_segments := split(path, "/") + # ─── Read-only operations ──────────────────────────────────────── allow_sandbox if { is_sandbox @@ -75,7 +112,7 @@ allow_sandbox if { allow_sandbox if { is_sandbox input.Method == "POST" - input.PathPlain == "/build" + path == "/build" } # `/session` is the CLI's BuildKit session endpoint: the daemon's built-in @@ -87,25 +124,26 @@ allow_sandbox if { allow_sandbox if { is_sandbox input.Method == "POST" - input.PathPlain == "/session" + path == "/session" } # ─── Image pulls ───────────────────────────────────────────────── allow_sandbox if { is_sandbox input.Method == "POST" - input.PathPlain == "/images/create" + path == "/images/create" } # ─── Container create — the project's label and a safe config ──── -# Equality on PathPlain matters twice: the daemon forwards a JSON body to the -# plugin for *any* endpoint whose content type is JSON, and the attach/exec -# handlers ignore fields they do not know — so a rule matching a path prefix -# could be satisfied by POST /containers//attach with a crafted body. +# Equality on the version-free path matters twice: the daemon forwards a JSON +# body to the plugin for *any* endpoint whose content type is JSON, and the +# attach/exec handlers ignore fields they do not know — so a rule matching a +# path prefix could be satisfied by POST /containers//attach with a crafted +# body. allow_sandbox if { is_sandbox input.Method == "POST" - input.PathPlain == "/containers/create" + path == "/containers/create" project_container safe_container_config } @@ -116,7 +154,7 @@ allow_sandbox if { allow_sandbox if { is_sandbox input.Method == "POST" - input.PathPlain == "/containers/create" + path == "/containers/create" testcontainers_container safe_container_config } @@ -291,7 +329,7 @@ host_config := object.get(input, ["Body", "HostConfig"], {}) allow_sandbox if { is_sandbox input.Method == "POST" - startswith(input.PathPlain, "/containers/") + startswith(path, "/containers/") lifecycle_action } @@ -300,14 +338,14 @@ lifecycle_action if { "start", "stop", "restart", "kill", "pause", "unpause", "wait", "update", } - endswith(input.PathPlain, sprintf("/%s", [action])) + endswith(path, sprintf("/%s", [action])) } # ─── Container delete (documented limitation, see Limitations) ─── allow_sandbox if { is_sandbox input.Method == "DELETE" - startswith(input.PathPlain, "/containers/") + startswith(path, "/containers/") } # ─── Network and volume creation ───────────────────────────────── @@ -318,14 +356,14 @@ allow_sandbox if { allow_sandbox if { is_sandbox input.Method == "POST" - input.PathPlain == "/networks/create" + path == "/networks/create" project_named_body } allow_sandbox if { is_sandbox input.Method == "POST" - input.PathPlain == "/volumes/create" + path == "/volumes/create" project_named_body safe_volume_create } @@ -358,14 +396,14 @@ volume_driver_ok if { allow_sandbox if { is_sandbox input.Method == "POST" - startswith(input.PathPlain, "/networks/") + startswith(path, "/networks/") project_path_resource } allow_sandbox if { is_sandbox input.Method == "POST" - startswith(input.PathPlain, "/volumes/") + startswith(path, "/volumes/") project_path_resource } @@ -377,14 +415,14 @@ allow_sandbox if { allow_sandbox if { is_sandbox input.Method == "DELETE" - startswith(input.PathPlain, "/networks/") + startswith(path, "/networks/") project_path_resource } allow_sandbox if { is_sandbox input.Method == "DELETE" - startswith(input.PathPlain, "/volumes/") + startswith(path, "/volumes/") project_path_resource } @@ -401,16 +439,16 @@ project_named_body if { startswith(input.Body.Name, "PROJECT_NAME_") } -# The plugin enriches the input with PathArr (the request path split on "/"), -# so a resource named in the path is matched as a whole segment rather than as -# a substring of the path, which also means the query string cannot satisfy it. +# A resource named in the path is matched as a whole segment of the derived +# path rather than as a substring of it, which also means neither the version +# segment nor a query string can satisfy the match. project_path_resource if { - some segment in input.PathArr + some segment in path_segments segment == "PROJECT_NAME" } project_path_resource if { - some segment in input.PathArr + some segment in path_segments startswith(segment, "PROJECT_NAME_") } diff --git a/schematics/authorize-docker-requests/skeleton/agent.rego.schema b/schematics/authorize-docker-requests/skeleton/agent.rego.schema index b90a4e3..d82eaac 100644 --- a/schematics/authorize-docker-requests/skeleton/agent.rego.schema +++ b/schematics/authorize-docker-requests/skeleton/agent.rego.schema @@ -82,13 +82,30 @@ The plugin enriches the request the daemon passes it (Docker's | `input.AuthMethod` | string | `TLS` when the client authenticated with a certificate | | `input.Method` | string | HTTP method (`GET`, `HEAD`, `POST`, `DELETE`) | | `input.Path` | string | Request path with the API version prefix and query string | -| `input.PathPlain` | string | Request path without the query string (plugin addition) | -| `input.PathArr` | array | `PathPlain` split on `/` (plugin addition) | +| `input.PathPlain` | string | The **raw request path**: API version prefix included, query string excluded (`u.Path`). `/v1.56/containers/create`, not `/containers/create` — nothing strips the version (issue #35), which is why the policy derives `path` from it (R-19) instead of matching the field | +| `input.PathArr` | array | `PathPlain` split on `/` — `["", "v1.56", "containers", "create"]` (plugin addition). Not read by the policy: `path_segments` is the derived path split the same way, so the version element cannot reach a match | +| `input.Query` | object | The parsed query string, as a map of arrays (`{"name": ["web-1"]}`). A container create's name arrives here, not in the body; no grant depends on it | | `input.Headers` | object | Request headers as a flat `string → string` map — Docker's own message type is `map[string]string`, so a value is a string, never an array | | `input.Body` | object | Decoded JSON request body, or `null` for requests without one (a `DELETE` carries none) | | `input.Body.HostConfig` | object | The create's host-side configuration — `Privileged`, `CapAdd`, `Devices`, `SecurityOpt`, `VolumesFrom`, `PidMode`, `IpcMode`, `NetworkMode`, `CgroupnsMode`, `UsernsMode`, `Binds`, `Mounts` — and the whole of the R-15 gate | | `input.BindMounts` | array | One object per bind mount of a container-create request: `Source`, `ReadOnly`, `Resolved` (plugin addition) | +Every path the policy matches is the derived `path` — `PathPlain` with one +optional `/v[.]` prefix removed, and with no `..` segment: + +```rego +path := p if { + p := regex.replace(object.get(input, "PathPlain", ""), "^/v[0-9]+(\\.[0-9]+)?", "") + not traversal(p) +} +path_segments := split(path, "/") +``` + +`PathPlain` itself is read for nothing else. Fed the real field, this decides the +same as it does for a client that omits the version; fed a version-less field +(what this package shipped in 0.4.0), every equality rule matched — which is why +a probe table written against that shape proved nothing about a live daemon. + `Resolved` is the source path with symlinks resolved, and it is the field that defeats the documented symlink bypass of bind-mount checks — but the plugin can only resolve paths it can read. A managed-plugin install mounts only the policy @@ -115,13 +132,41 @@ shape. The decisions this package relies on, each an `opa eval` against the deployed file with a hand-built input. Every row below was run under OPA v0.60.0, v1.3.0, and v1.7.1 (the two embedded engines plus a newer one), and every row decides as -its Expected column says on all three. Run them before trusting a change: a -policy that loads is not a policy that decides correctly. Thirty of the rows -decided wrongly before 0.4.0: 29 were "allow" where the answer is deny — -R15.01–R15.18, R15.25, R15.28, R15.29, R15.33, R15.35, R16.01, R16.05, R18.02, -R18.04, R18.05 and `/build/prune` — and one, `POST /session`, was denied where -the answer is allow. The rest were already correct, and are listed so a future -change cannot quietly break them. +its Expected column says on all three. + +**The inputs carry the plugin's real field values** — `Path` and `PathPlain` +with the API version prefix, `PathArr` split from that, `Query` parsed — because +the version-less shape this package used until 0.5.0 is not what `main.go`'s +`makeInput` produces, and a table written against it says nothing about a live +daemon. Concretely, the two shape errors this table is built to catch: + +- **With a version-less `PathPlain`** (the 0.4.0 table), every host-access row + passed while the policy was allowing them, and `/session` looked denied: 30 + rows decided wrongly, 29 of them "allow" where the answer is deny + (R15.01–R15.18, R15.25, R15.28, R15.29, R15.33, R15.35, R16.01, R16.05, + R18.02, R18.04, R18.05, `/build/prune`). +- **With the real `PathPlain`** (this table), the same 0.4.0 policy decided + wrongly on 26 rows — every one of them a legitimate request *denied*: + creates, volume and network creates, deletes, lifecycle actions, build, pull, + `/session`. That is issue #35: the hardening's equality grants matched + nothing, and the failure was invisible because it failed closed. + +Each row is one `opa eval` against the substituted policy: + +``` +opa eval -f raw --data agent.rego --input .json data.docker.authz.allow +``` + +evaluated with the engine the plugin embeds (P-10) — a container image of that +engine does when no matching `opa` binary is available. The version prefix the +rows carry is a parameter of whatever builds them, not of the policy: run the +table twice when the shape is in question, once with a prefix and once without, +and require identical decisions. Then run at least one **live** test (SCHEMATIC.md's A-11 or +A-16): a table cannot disagree with the plugin if it is fed the plugin's values, +but nothing but a live call proves those values are the plugin's. + +The rows are also a regression suite: they are listed so a future change cannot +quietly break them. The `` token below stands for the project name (`backend-services` with the defaults), `` for the project directory (`/srv/compose/backend-services`), @@ -178,6 +223,13 @@ otherwise. | `POST /build/prune` | deny — no match by prefix | | `POST /session` | allow — the BuildKit CLI session R-7's build needs | | `POST /containers/create?name=buildx_buildkit_default0`, no label | deny — a `name` query parameter attributes nothing, and the carve-out is gone | +| `GET /_ping` with no version prefix at all | allow — read-only, and nothing to strip (a `HEAD /_ping` never reaches the policy: `-skip-ping` defaults to true) | +| `POST /containers/create` with the project label, **no** version prefix | allow — a client that omits the version is not a different client | +| `POST /v1.56/v1.56/containers/create` with the project label | deny — one strip only, and the result is not a granted path | +| `POST /v1.56/containers/../containers/create` with the project label | deny — a `..` segment means no `path`, so no rule matches | +| `POST /v1.56/containers/../../volumes/_data` | deny — same guard, on a path-named volume operation | +| `DELETE /v1.56/volumes/../volumes/hostdata` | deny — same guard, on a delete | +| `POST /v1/containers/create` with the project label (major only) | allow — the minor is optional in the strip; the daemon's own router still decides whether it routes | ### R-12 — creation is scoped by name From 6eccbb3adc208f59768e0238e23507d2f3956ab3 Mon Sep 17 00:00:00 2001 From: phoenix-server Date: Thu, 17 Sep 2026 19:09:38 -0400 Subject: [PATCH 2/2] authorize-docker-requests: state current facts, not the history that produced them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The package's prose carried version-to-version narrative — which release shipped which probe table, what a field used to be, which rule a defect produced — and issue numbers used as the reason for a rule. A schematic's reader is a builder who has the artifact and nothing else; that history belongs in the PR, the issue and the changelog, and it costs tokens without changing what the builder does. Every such sentence is now the current requirement, keeping what a builder needs: the trap (a probe row must carry the plugin's real field values, because a version-less `PathPlain` proves nothing about a live daemon), the compatibility facts (which plugin tags embed which OPA engine, which reject `import rego.v1`), and the verification provenance (78 rows, run under OPA v0.60.0, v1.3.0 and v1.7.1). A reverse-engineering note that only narrated the defect is removed; the input contract it described stays stated three times where a builder reads it — as R-19, in the Rego input schema, and as the probe table's own rule. The Decisions entry is a decision list again (the derivation, why not rebuild from `PathArr`, the `..` guard, the reload being a file replacement, the parameter discoveries) rather than a report of one. Comments only in skeleton/agent.rego: the policy code is unchanged, verified by comparing the files with comments stripped. --- .../authorize-docker-requests/SCHEMATIC.md | 160 +++++++----------- .../modules/opa-policy.md | 19 +-- .../modules/policy-reload.md | 14 +- .../skeleton/agent.rego | 6 +- .../skeleton/agent.rego.schema | 46 ++--- 5 files changed, 102 insertions(+), 143 deletions(-) diff --git a/schematics/authorize-docker-requests/SCHEMATIC.md b/schematics/authorize-docker-requests/SCHEMATIC.md index ec0d7cd..951a4f2 100644 --- a/schematics/authorize-docker-requests/SCHEMATIC.md +++ b/schematics/authorize-docker-requests/SCHEMATIC.md @@ -40,18 +40,6 @@ updated: 2026-09-17 > table in which 30 of its 71 rows decided wrongly before the change and every > row decides as specified after it, on the three OPA engines the package names > (see `skeleton/agent.rego.schema`). -> -> **2026-09-17 — the path contract was wrong, and the hardening was dead on a -> live daemon.** `PathPlain` carries the API version prefix (`"PathPlain": -> u.Path` in the plugin's `main.go`), so every rule the hardening keyed on that -> field by equality — the create, build, pull, volume and network grants — -> matched nothing on a real host, and the sandbox could not create anything for -> its own project. It failed **closed**, which is why the probe table did not -> catch it: the table was the only test, and its inputs were the documented -> shape, which was itself wrong. The policy now derives a version-free path -> (R-19) and the table's inputs are the plugin's real ones; see `main.go`'s -> `makeInput` for the two fields that differ. - Grants a sandbox container (isolated agent, CI runner, or untrusted workload) restricted TCP access to the host Docker daemon, policed by Open Policy Agent. After implementing this schematic, the host's Docker daemon listens on a TLS @@ -225,11 +213,10 @@ deployment that does not accept them is deploying something else. running engines and probes as described in `skeleton/agent.rego.schema` — 78 rows, decision-checked on three OPA engines (OPA v0.60.0 and v1.3.0, the two the installable plugin releases embed, plus v1.7.1) — but a row decides only - about the input it carries. The 71-row table that shipped in 0.4.0 used a - version-less `PathPlain`, which the plugin never sends, so it passed while - every create on a live daemon was denied (issue #35). The rows now carry the - plugin's real values; only a **live** run (A-11, A-16) can find a mismatch - between the table's model of the input and the plugin's. + about the input it carries. A row fed a version-less `PathPlain` proves + nothing: the plugin always sends the version, so the table agrees with itself + while the rules it covers decide nothing. Only a **live** run (A-11, A-16) + can find a mismatch between the table's model of the input and the plugin's. - **A project name that is also an API path segment widens the segment match.** `P-3` is matched as a whole path segment (R-12, R-18). A deployment whose project is called `containers` — the host this package was verified against — @@ -329,14 +316,13 @@ deployment that does not accept them is deploying something else. start, stop, restart, kill, pause, unpause, wait, update. - **R-9**: The sandbox container MUST NOT require Docker configuration changes when the OPA policy is updated — policy reload is a host-side operation. - **Revision 2026-09-17**: "reload" is now precisely stated, because the - procedure this package shipped was wrong twice. The plugin re-reads the - policy **file on every request** (`os.ReadFile(p.policyFile)` inside - `evaluatePolicyFile`), so a policy change is a file replacement and nothing - else: no plugin bounce (disabling a plugin the daemon references makes - dockerd **exit** — measured, Q-4), no daemon restart, and no sandbox change. - The replacement must be atomic, because a *missing* policy file is the - plugin's one fail-open path. See the policy-reload module. + "Reload" is precisely stated: the plugin re-reads the policy **file on every + request** (`os.ReadFile(p.policyFile)` inside `evaluatePolicyFile`), so a + policy change is a file replacement and nothing else — no plugin bounce + (disabling a plugin the daemon references makes dockerd **exit**; measured, + Q-4), no daemon restart, and no sandbox change. The replacement must be + atomic, because a *missing* policy file is the plugin's one fail-open path. + See the policy-reload module. - **R-10**: The host's unix socket (`/var/run/docker.sock`) MUST remain unrestricted for local host users. - **R-11**: The OPA policy MUST allow testcontainers-go containers — those @@ -416,7 +402,7 @@ deployment that does not accept them is deploying something else. carrying a `..` segment — the daemon cleans the path before routing while the plugin authorizes the raw one. The documented input shape, and every probe row, MUST use the values the plugin actually sends; a version-less probe - input proves nothing about a live daemon (issue #35). + input proves nothing about a live daemon. ## Design Principles Binding the Implementation @@ -520,8 +506,9 @@ for the `AuthZPlugin.AuthZReq` / `AuthZPlugin.AuthZRes` message schema. ### Rego input schema (contract consumed by the policy) The plugin passes the Docker API request to the Rego evaluation in this shape. -The values are the plugin's own, quoted from `main.go`'s `makeInput` — the -obvious-looking rewrite of two of them is what issue #35 was: +The values are the plugin's own, quoted from `main.go`'s `makeInput` — and +the two path fields are easy to misread: `PathPlain` keeps the API version +prefix, and `PathArr` is split from that same versioned path: ```go input := map[string]interface{}{ @@ -561,9 +548,9 @@ input := map[string]interface{}{ (`/v1.47/containers/create`), and a real plugin sends exactly that: nothing in `main.go` strips the prefix, and `-skip-ping` only bypasses `HEAD /_ping`. A policy that matches the raw field by equality therefore decides nothing on a -live daemon. That was issue #35 — the hardening closed the host-access holes -and, with them, every create, build, pull, delete and lifecycle grant — and it -failed closed, which is why it looked secure rather than broken. +live daemon, and it fails **closed**: the grants disappear while every denial +holds, so the sandbox cannot create, build, pull or delete anything for its own +project — a broken policy that looks like a working one. The policy derives the version-free path once (R-19) and every rule matches that: @@ -1102,10 +1089,10 @@ Each test names the requirements it covers. `...` stands for (`main.go`'s `makeInput`) unless it uses the plugin's values. Run the table **twice** where the shape is in question: once with a version prefix and once without, and require identical decisions. - **This test alone is not enough**, and #35 is the proof: with a wrong input - shape the whole table passed while every create on a live host was denied. The - live tests (A-11, A-16) are the ones that catch that class; run at least one - of them against a real daemon before trusting a policy change. + **This test alone is not enough**: with a wrong input shape the whole table + passes while every create on a live host is denied. The live tests (A-11, + A-16) are the ones that catch that class; run at least one of them against a + real daemon before trusting a policy change. - **A-16** (covers R-15, R-16): the host-access gate holds for a project-labelled client — from the sandbox, each of these is denied by the plugin, and the denial is not a mistake of syntax but the R-15/R-16 decision: @@ -1372,70 +1359,45 @@ Decisions: deletion of the per-package `SCHEMATIC.md.schema` is in the base rather than a conflict here. -- 2026-09-17 (later same day) — **The path contract was wrong: the hardening above was dead on a live daemon** (#35). Everything below is measured, not read. - - **The plugin's `PathPlain` carries the API version prefix.** `main.go`'s - `makeInput` sets `"PathPlain": u.Path` and `"PathArr": - strings.Split(u.Path, "/")` (line 257) from the raw request URL; nothing - strips a version (`-skip-ping` only bypasses `HEAD /_ping`). The decision - log of the running plugin shows it plainly: `"Path": - "/v1.56/containers/json"`, `"PathPlain": "/v1.56/containers/json"`, - `"PathArr": ["","v1.56","containers","json"]`. - - **So the equality grants R-17 introduced matched nothing** — and neither did - the `startswith` rules behind lifecycle actions, container deletion and - path-named network/volume calls. On a live daemon the sandbox could not - create a container, volume or network for its own project, and could not - start, stop or delete one. The documented input shape in this file showed a - version-less `PathPlain`, and the 0.4.0 probe table was built from that - shape, so the table agreed with a fiction. - - **It was invisible because it failed closed.** A sandbox that cannot create - anything looks secure rather than broken. Only a test that uses the real - plugin — or its real field values — can catch this class; the table alone - did not, and A-15 now states that limit. - - **The fix derives the version-free path inside the policy** (R-19): `path` - is `PathPlain` with one optional `/v[.]` prefix removed and - with no `..` segment, and `path_segments` is `split(path, "/")`, so - `PathArr` is no longer read at all. Deriving in the policy rather than - rebuilding from `PathArr` was chosen because rebuilding mishandles a path - with no version (`/_ping` would lose a real segment) and needs the same - guard anyway, while an anchored regex leaves an unversioned path untouched — - a client that omits the version keeps working. The `..` guard is new: the - daemon cleans the path before routing while the plugin authorizes the raw - one, so no rule should be reachable through a traversal. - - **Evidence, in the real input shape** (`Path` and `PathPlain` the raw - versioned path, `PathArr` split from that, `Query` parsed) — 78 rows on - three engines: the 0.4.0 policy decided wrongly on 26 of them, every one a - legitimate request **denied** (creates, network and volume creates, deletes, - lifecycle actions, build, pull, `/session`); the fixed policy decides all 78 - as specified on OPA v0.60.0, v1.3.0 and v1.7.1. The same table run with the - version prefix removed, and with `v1.43`, gives identical decisions — the - property R-19 states. - - **Version 0.5.0 (minor)**: R-19 is a *new* requirement (path matching must - be version-independent), not a re-wording of an existing one; the defect - alone would have been a patch. - - **Q-4 answered — and the reload procedure was wrong twice.** Disabling a - plugin the running daemon references is FATAL: `level=fatal msg="Error - validating authorization plugin" error="plugin \"\" not found"`, and - dockerd exits (measured on Docker 29.6.1, snap install). In that state - `docker plugin enable` cannot help, because it needs a running daemon — the - recovery is to remove the reference from the live configuration file (P-13) - first. But the bounce was never *needed*: `evaluatePolicyFile` reads the - policy **file on every request**, so the deployed file *is* the live policy. - The procedure is now a file replacement — install to a temporary name, then - `mv -f` over the target — because a *missing* policy file is the plugin's - one fail-**open** path (`OPA policy file %s does not exist, failing open and - allowing request`), which `install -D` over the live path could create. - `scripts/reload-opa-policy.sh` no longer touches plugin state, and verifies - a deployment through the decision log's `config_hash` instead. - - **Q-5 answered — yes**: a snap-installed daemon does bind-mount host `P-6` - into the managed plugin at `/opa`; verified by the operator on the snap host - this package was tested against. - - **Deployment note from the tested host**: the plugin registers as - `opa-docker-authz:latest` (installed without `--alias`), which is the name - the daemon's `authorization-plugins` entry must carry, and the name that - appears in denial messages. P-14 is a parameter for exactly this reason. - - **Residual**: the live end-to-end pass (A-11, A-16) belongs to the operator - against a real daemon; this package's evidence is the table above plus the - measured plugin behaviour it rests on. +- **Path matching derives the version-free path inside the policy** (R-19): + `path` is `PathPlain` with one optional `/v[.]` prefix removed + and with no `..` segment, and `path_segments` is `split(path, "/")`. Deriving + was chosen over rebuilding the path from `PathArr` because rebuilding + mishandles a path that carries no version (`/_ping` would lose a real segment) + and needs the same guard anyway, while one anchored regex leaves an + unversioned path untouched — a client that omits the version keeps working. + The `..` guard exists because the daemon cleans the path before routing while + the plugin authorizes the raw one, so no rule may be reachable through a + traversal. The property is asserted by running the probe table with a version + prefix, without one, and with a major-only prefix: all three give identical + decisions. +- **A policy reload replaces the file and never touches the plugin (Q-4).** + Disabling, removing or upgrading a plugin that a running daemon references is + fatal: `level=fatal msg="Error validating authorization plugin" error="plugin + \"\" not found"`, and dockerd exits (measured on Docker 29.6.1, snap + install). `docker plugin enable` cannot recover that state, because it needs a + running daemon — remove the reference from the live configuration file (P-13) + first. No bounce is needed in any case: `evaluatePolicyFile` reads the policy + **file on every request**, so the deployed file *is* the live policy. Replace + it by installing to a temporary name and renaming over the target, because a + *missing* policy file is the plugin's one fail-open path (`OPA policy file %s + does not exist, failing open and allowing request`), which copying or + truncating the live path can produce. + `scripts/reload-opa-policy.sh` does this and verifies the result through the + decision log's `config_hash`. +- **A snap-installed daemon does bind-mount the policy directory (Q-5).** The + host directory (P-6) appears inside the managed plugin at `/opa`, so the + policy is a host file the plugin reads. The plugin's own record names both + halves — `docker plugin inspect ` shows the `-policy-file` argument and + the mount that carries it — which is how the deploy script derives the host + path (P-7) instead of assuming it. +- **The registered plugin name is the install name, tag included** (P-14). + Installed without `--alias` it registers as `opa-docker-authz:latest`, which + is the name the daemon's `authorization-plugins` entry must carry and the name + that appears in denial messages. +- **The probe table's evidence is decision-level.** A live end-to-end pass + (A-11, A-16) against a real daemon is what proves the table's model of the + input matches the plugin's. Open questions: diff --git a/schematics/authorize-docker-requests/modules/opa-policy.md b/schematics/authorize-docker-requests/modules/opa-policy.md index db879bc..7798478 100644 --- a/schematics/authorize-docker-requests/modules/opa-policy.md +++ b/schematics/authorize-docker-requests/modules/opa-policy.md @@ -51,7 +51,7 @@ before evaluation. Fields the policy uses: - `input.Method` — string. HTTP method: `GET`, `HEAD`, `POST`, `DELETE`. - `input.PathPlain` — string. The **raw request path**: API version prefix included, query string excluded (`u.Path`), e.g. `/v1.56/containers/json`. - Nothing strips the version (issue #35), so the policy derives `path` from it + Nothing strips the version, so the policy derives `path` from it and matches that with `==` (R-17, R-19) — the raw field is read for nothing else. - `input.PathArr` — array. `PathPlain` split into path elements, so its second @@ -127,9 +127,9 @@ A leftover is a silent full-access bug, not a cosmetic one: `is_sandbox` then never matches a real client, so the sandbox is classified as a host user and every request is allowed. See Phase 6 and its acceptance test. -(`P-11`'s `BUILDKIT_PREFIX` token is retired — R-17 removed the BuildKit -carve-out — but the leftover check still greps for it, so deploying a copy of -the pre-R-17 template is caught rather than silently accepted.) +(`P-11`'s `BUILDKIT_PREFIX` token is not read by any rule, but the leftover +check still greps for it, so a copy that carries the token is rejected rather +than silently deployed.) ## Limitations @@ -152,10 +152,10 @@ limitations): deliberately, and the probe table must grow a row with it. - **The table is not the daemon.** A probe row decides about the input it carries; if that input is not the plugin's, the table agrees with a fiction. - Until 0.5.0 every row carried a version-less `PathPlain` that the plugin never - sends, so the table passed while live creates were denied (issue #35). Rows - now use the plugin's values (`main.go`'s `makeInput`), and the live tests are - what prove the values are the plugin's. + The plugin always sends the API version prefix in `PathPlain`, so a row fed a + version-less path exercises a request that never arrives — run rows with the + plugin's own values (`main.go`'s `makeInput`), and treat a live test as what + proves those values are the plugin's. - **Host port publishing is not part of the gate.** A project container may publish a host port (`ports:`), which does not read the host filesystem but can occupy a free port and answer for it. Closing that is the daemon @@ -178,8 +178,7 @@ limitations): ## Dependencies - D-1, D-3, D-4 (from SCHEMATIC.md) -- Parameters P-3, P-4, P-9, P-12, P-15 (P-11 is retired with the BuildKit - carve-out, R-17) +- Parameters P-3, P-4, P-9, P-12, P-15 (P-11 is not read by this policy) ## Failure Behavior diff --git a/schematics/authorize-docker-requests/modules/policy-reload.md b/schematics/authorize-docker-requests/modules/policy-reload.md index 3ed662c..e3e6a26 100644 --- a/schematics/authorize-docker-requests/modules/policy-reload.md +++ b/schematics/authorize-docker-requests/modules/policy-reload.md @@ -33,8 +33,7 @@ code decide the whole procedure: the running daemon references makes dockerd treat its own configuration as invalid and **exit**: `level=fatal msg="Error validating authorization plugin" error="plugin \"\" not found"` (measured on Docker 29.6.1, snap - install; Q-4 in SCHEMATIC.md's decisions). Before this was measured, the - script this package shipped did exactly that. + install; Q-4 in SCHEMATIC.md's decisions). - **It is not removing the plugin reference.** Deleting `P-14` from `authorization-plugins` and sending SIGHUP leaves an unrestricted daemon: every request is allowed while the entry is absent. That is the deliberate @@ -59,18 +58,17 @@ code decide the whole procedure: ## Pre-flight checks (before touching the deployed file) -1. **No placeholders left**: `grep -nE 'SANDBOX_USERNAME|AUTH_HEADER_NAME|PROJECT_NAME|PROJECT_DIR_PATH|BUILDKIT_PREFIX|TESTCONTAINERS_LABEL' ` must print nothing. A leftover token would be deployed as a literal and silently turn the policy into "allow everything". (`BUILDKIT_PREFIX` is retired — R-17 removed the BuildKit carve-out — and is kept in this pattern on purpose: the check is a superset of the template's tokens, so a source copied from an older template is still caught.) +1. **No placeholders left**: `grep -nE 'SANDBOX_USERNAME|AUTH_HEADER_NAME|PROJECT_NAME|PROJECT_DIR_PATH|BUILDKIT_PREFIX|TESTCONTAINERS_LABEL' ` must print nothing. A leftover token would be deployed as a literal and silently turn the policy into "allow everything". (`BUILDKIT_PREFIX` is not read by the policy and is kept in this pattern on purpose: the check is a superset of the template's tokens, so a source that carries the token is caught rather than deployed.) 2. **It parses**, under an engine **no newer than the plugin's** (P-10): `opa check `, or the engine's own image when no binary is installed — `docker run --rm -v "$(pwd):/w:ro" -w /w openpolicyagent/opa:1.3.0 check `. A newer engine accepts syntax the plugin's engine rejects. 3. **It decides correctly**: the `opa eval` probes in `skeleton/agent.rego.schema` still produce the expected allow/deny results — - **with the plugin's real input shape**, which is the part this package got - wrong once: `PathPlain` carries the API version prefix (`"PathPlain": u.Path` - in `main.go`), so a probe table written with a version-less `PathPlain` - proves nothing about a live daemon (issue #35). Every path in the table is - `/v1./…`, and the runner records the version it used. + **with the plugin's real input shape**: `PathPlain` carries the API version + prefix (`"PathPlain": u.Path` in `main.go`), so a probe table written with a + version-less `PathPlain` proves nothing about a live daemon. Every path in + the table is `/v1./…`, and the runner records the version it used. 4. **A copy of the currently deployed policy is kept** before the replacement, so the change can be reverted with the same procedure. diff --git a/schematics/authorize-docker-requests/skeleton/agent.rego b/schematics/authorize-docker-requests/skeleton/agent.rego index 63c3eb1..52a7289 100644 --- a/schematics/authorize-docker-requests/skeleton/agent.rego +++ b/schematics/authorize-docker-requests/skeleton/agent.rego @@ -16,9 +16,9 @@ package docker.authz # TESTCONTAINERS_LABEL_KEY → P-12 label key, e.g. "org.testcontainers" # TESTCONTAINERS_LABEL_VALUE → P-12 label value, e.g. "true" # -# (P-11 BUILDKIT_PREFIX is superseded: the BuildKit carve-out is gone, see -# SCHEMATIC.md's Decisions. The leftover-token check in the package still greps -# for the token, so a template carrying the old carve-out is still caught.) +# (P-11 BUILDKIT_PREFIX is not read by any rule — see SCHEMATIC.md's Decisions. +# The leftover-token check in the package still greps for it, so a copy carrying +# the token is caught rather than deployed.) # # Language and engine: Rego v1 syntax, evaluated by the OPA engine embedded in # the plugin. The plugin release determines the engine (see P-10), measured from diff --git a/schematics/authorize-docker-requests/skeleton/agent.rego.schema b/schematics/authorize-docker-requests/skeleton/agent.rego.schema index d82eaac..d9ea4ea 100644 --- a/schematics/authorize-docker-requests/skeleton/agent.rego.schema +++ b/schematics/authorize-docker-requests/skeleton/agent.rego.schema @@ -57,9 +57,8 @@ request is allowed. | `TESTCONTAINERS_LABEL_KEY` | P-12 | `org.testcontainers` | | `TESTCONTAINERS_LABEL_VALUE` | P-12 | `true` | -`BUILDKIT_PREFIX` (P-11) is **retired**: R-17 removed the BuildKit carve-out, so -nothing reads that token and it is gone from the file. The leftover check below -still greps for it, so deploying a copy of the pre-0.4.0 template is caught +`BUILDKIT_PREFIX` (P-11) is not read by this file: no rule uses it. The leftover +check below still greps for it, so a copy that carries the token is caught rather than silently accepted. The deployed copy must be checked, not assumed — and the check must ignore @@ -82,7 +81,7 @@ The plugin enriches the request the daemon passes it (Docker's | `input.AuthMethod` | string | `TLS` when the client authenticated with a certificate | | `input.Method` | string | HTTP method (`GET`, `HEAD`, `POST`, `DELETE`) | | `input.Path` | string | Request path with the API version prefix and query string | -| `input.PathPlain` | string | The **raw request path**: API version prefix included, query string excluded (`u.Path`). `/v1.56/containers/create`, not `/containers/create` — nothing strips the version (issue #35), which is why the policy derives `path` from it (R-19) instead of matching the field | +| `input.PathPlain` | string | The **raw request path**: API version prefix included, query string excluded (`u.Path`). `/v1.56/containers/create`, not `/containers/create` — nothing strips the version, which is why the policy matches the derived `path` (R-19) instead of this field | | `input.PathArr` | array | `PathPlain` split on `/` — `["", "v1.56", "containers", "create"]` (plugin addition). Not read by the policy: `path_segments` is the derived path split the same way, so the version element cannot reach a match | | `input.Query` | object | The parsed query string, as a map of arrays (`{"name": ["web-1"]}`). A container create's name arrives here, not in the body; no grant depends on it | | `input.Headers` | object | Request headers as a flat `string → string` map — Docker's own message type is `map[string]string`, so a value is a string, never an array | @@ -102,9 +101,9 @@ path_segments := split(path, "/") ``` `PathPlain` itself is read for nothing else. Fed the real field, this decides the -same as it does for a client that omits the version; fed a version-less field -(what this package shipped in 0.4.0), every equality rule matched — which is why -a probe table written against that shape proved nothing about a live daemon. +same as it does for a client that omits the version; fed a version-less field, +the equality rules match nothing — which is why a probe table written against +that shape proves nothing about a live daemon. `Resolved` is the source path with symlinks resolved, and it is the field that defeats the documented symlink bypass of bind-mount checks — but the plugin can @@ -134,22 +133,23 @@ file with a hand-built input. Every row below was run under OPA v0.60.0, v1.3.0, and v1.7.1 (the two embedded engines plus a newer one), and every row decides as its Expected column says on all three. -**The inputs carry the plugin's real field values** — `Path` and `PathPlain` -with the API version prefix, `PathArr` split from that, `Query` parsed — because -the version-less shape this package used until 0.5.0 is not what `main.go`'s -`makeInput` produces, and a table written against it says nothing about a live -daemon. Concretely, the two shape errors this table is built to catch: - -- **With a version-less `PathPlain`** (the 0.4.0 table), every host-access row - passed while the policy was allowing them, and `/session` looked denied: 30 - rows decided wrongly, 29 of them "allow" where the answer is deny - (R15.01–R15.18, R15.25, R15.28, R15.29, R15.33, R15.35, R16.01, R16.05, - R18.02, R18.04, R18.05, `/build/prune`). -- **With the real `PathPlain`** (this table), the same 0.4.0 policy decided - wrongly on 26 rows — every one of them a legitimate request *denied*: - creates, volume and network creates, deletes, lifecycle actions, build, pull, - `/session`. That is issue #35: the hardening's equality grants matched - nothing, and the failure was invisible because it failed closed. +**Every row's input carries the plugin's real field values** — `Path` and +`PathPlain` with the API version prefix, `PathArr` split from that, `Query` +parsed — because `main.go`'s `makeInput` builds them from the raw request URL +and nothing strips the version. A table built on a version-less `PathPlain` +proves nothing about a live daemon, and the two shapes disagree in both +directions: + +- **A version-less `PathPlain`** makes an equality rule match a path the plugin + never sends, so rows the policy must deny appear allowed, and the rows that + show the grant working are absent. +- **The real `PathPlain`** leaves every equality rule unmatched, so the grants + vanish: creates, volume and network creates, deletes, lifecycle actions, + build, pull and `/session` all decide `false`, and the failure is easy to miss + because it fails **closed** — every denial still holds. + +Run the table whenever the shape is in doubt, and confirm the Expected column +of **every** row; a table that agrees with itself is not evidence. Each row is one `opa eval` against the substituted policy: