From d14516a74d9bb393e9b2aee78b8c9e9d99835daf Mon Sep 17 00:00:00 2001 From: ZDDC Date: Tue, 9 Jun 2026 19:30:09 -0500 Subject: [PATCH] fix(server): fail-close the reference Rego; stop claiming internal-decider parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bundled reference Rego (`zddc-server --print-rego`) modeled the read-ACL cascade only, but its header claimed to "mirror the internal decider exactly, validated on every CI run." It is verb-blind, role-blind, WORM-blind, and admin-blind: an external-OPA deployment (ZDDC_OPA_URL=http(s)/unix) loading it granted writes/deletes to read-only principals and ignored WORM zones. The parity tests never exercised a write action, a role principal, a WORM level, or is_active_admin — so the divergence shipped silently behind a false "mirrors exactly" claim. Make both shipped policies fail-closed instead of falsely-complete: - access.rego / access_federal.rego: gate every cascade grant on a read action (empty/absent == read); non-read actions fall through to default-deny. access.rego honors the single is_active_admin bypass (the one write-capable principal); access_federal.rego deliberately has none (strict AC-6). - Rewrite the access.rego / access_federal.rego / rego.go headers: these are read-ACL SKELETONS, NOT a tested mirror of the internal decider; operators must add write/WORM/role/admin semantics before granting writes. - policy.go: fix the stale AllowInput doc claiming the internal decider "treats read and write identically — any allow grants full CRUD" (it honors the action verb, with the WORM clamp and admin/elevation bypass applied). Tests: - rego_failclosed_test.go: pins the contract — reads allowed, every write verb denied, active-admin writes allowed (commercial) / denied (federal). - embedded_neutral_test.go: pins that EmbeddedDefaults() carries no top-level worm: and no role members — the invariant that makes policy.SerializableChain dropping PolicyChain.Embedded behavior-neutral (a latent wire-contract gap). Existing read-cascade parity + federal-divergence tests stay green; full Go suite + vet pass. The default in-process InternalDecider is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- zddc/internal/policy/policy.go | 17 ++-- zddc/internal/policy/rego.go | 18 ++-- zddc/internal/policy/rego/access.rego | 58 ++++++++--- zddc/internal/policy/rego/access_federal.rego | 65 +++++++----- zddc/internal/policy/rego_failclosed_test.go | 98 +++++++++++++++++++ zddc/internal/zddc/embedded_neutral_test.go | 45 +++++++++ 6 files changed, 251 insertions(+), 50 deletions(-) create mode 100644 zddc/internal/policy/rego_failclosed_test.go create mode 100644 zddc/internal/zddc/embedded_neutral_test.go diff --git a/zddc/internal/policy/policy.go b/zddc/internal/policy/policy.go index ac44fef..66cf95d 100644 --- a/zddc/internal/policy/policy.go +++ b/zddc/internal/policy/policy.go @@ -58,18 +58,21 @@ import ( // External Rego policies can: // - read input.user.email (string) // - read input.path (string) -// - read input.action ("read" | "write"); empty/absent ≡ "read" +// - read input.action ("read"|"write"|"create"|"delete"|"admin"); +// empty/absent ≡ "read" // - walk input.policy_chain.levels[].acl.{allow,deny} for // custom cascade semantics, or read the pre-resolved // input.policy_chain.has_any_file when implementing the // same default-deny rule we use internally. // -// Action distinguishes read (GET/HEAD on listings, files, app HTML) -// from write (PUT, DELETE, POST/move on the file API). The internal -// decider treats both identically — any allow grants full CRUD, -// matching the model in place before the file API existed (anyone -// with read access also had OS-level write via the mounted share). -// External Rego policies can split the two by inspecting input.action. +// Action distinguishes read (GET/HEAD on listings, files, app HTML) from +// write/create/delete/admin (PUT, DELETE, POST/move on the file API). The +// internal decider HONORS it: actionVerb maps each action to the verb it +// requires, and AllowActionFromChainP checks that specific verb against the +// cascade's effective grant, with the WORM clamp and the admin/elevation +// bypass applied. The bundled reference Rego, by contrast, models the +// read-ACL cascade only and is fail-closed for non-read actions — see +// rego/access.rego. type AllowInput struct { User struct { Email string `json:"email"` diff --git a/zddc/internal/policy/rego.go b/zddc/internal/policy/rego.go index 21f6ae3..9ff6e99 100644 --- a/zddc/internal/policy/rego.go +++ b/zddc/internal/policy/rego.go @@ -2,13 +2,19 @@ package policy import _ "embed" -// ReferenceRego is the canonical Rego policy bundled with zddc-server. -// It mirrors the InternalDecider's semantics exactly — every release CI -// run validates parity via parity_test.go (which imports the OPA library -// as a test-only dependency, so the production binary stays OPA-free). +// ReferenceRego is a read-ACL Rego SKELETON bundled with zddc-server for +// external-OPA deployments. It models the read cascade ONLY and is NOT a +// semantic mirror of the InternalDecider: it does not implement per-verb +// authorization (write/create/delete/admin), WORM zones, roles, fences, or +// config-edit, so it is FAIL-CLOSED — every non-read action is denied except +// for an elevated admin (input.user.is_active_admin). The InternalDecider +// remains the production source of truth. parity_test.go (OPA as a test-only +// dependency, so the production binary stays OPA-free) checks the modelled +// read-cascade dimension only — it does NOT prove full parity. // -// Operators running an external OPA can use this as the starting point -// for their own policy bundle: +// Operators running an external OPA can use this as a STARTING POINT — they +// must add the unmodelled write/WORM/role/admin semantics before relying on +// it for write authorization: // // zddc-server --print-rego > /etc/opa/policies/zddc-access.rego // diff --git a/zddc/internal/policy/rego/access.rego b/zddc/internal/policy/rego/access.rego index 8f9ba24..c691a6d 100644 --- a/zddc/internal/policy/rego/access.rego +++ b/zddc/internal/policy/rego/access.rego @@ -1,16 +1,26 @@ -# Reference Rego policy that mirrors zddc-server's built-in `internal` -# decider exactly. Federal customers running their own OPA can use this -# as a starting point (and then tighten — e.g. flip the leaf-allow-overrides- -# parent-deny rule for NIST AC-6 compliance). +# Reference Rego SKELETON for an external-OPA deployment. It models the +# read-ACL cascade ONLY. It is NOT semantically equivalent to zddc-server's +# built-in `internal` decider and MUST NOT be deployed as-is for a system +# that relies on write authorization. # -# The internal evaluator (in zddc/internal/zddc/acl.go) is the source of -# truth for production. This file is validated against that evaluator on -# every CI run via the parity test in zddc/internal/policy/parity_test.go. -# Both implementations must produce the same decision for every fixture. +# Models: the deepest-matching-level read-ACL cascade — glob patterns, +# deny-first-within-a-level, default-deny once any .zddc exists. +# +# Does NOT model (the internal decider in zddc/internal/zddc + internal/policy +# does): per-verb authorization (write/create/delete/admin), WORM zones, +# roles: membership resolution, inherit:false fences, and standing config-edit +# (the `a` verb). Because those are unmodelled this policy is FAIL-CLOSED: +# every non-read action is denied, and an elevated admin +# (input.user.is_active_admin) is the only write-capable principal. A real +# deployment must add the missing semantics before granting writes — see the +# parity tests under zddc/internal/policy for the dimensions to cover. The +# internal Go decider remains the production source of truth; this file is a +# starting point, not a tested mirror of it. # # Input shape (matches zddc/internal/policy.AllowInput JSON encoding): # { -# "user": {"email": "alice@example.com"}, +# "user": {"email": "alice@example.com", "is_active_admin": false}, +# "action": "read", # "" / absent == read; else write|create|delete|admin # "path": "/Project-A/sub/", # "policy_chain": { # "levels": [ @@ -39,14 +49,40 @@ import future.keywords.in default allow := false -# Allow when no .zddc files anywhere in the chain AND no rule matches. +# Elevated admins bypass — mirrors the internal decider's single admin +# short-circuit. The caller computes is_active_admin (admin authority on this +# chain AND elevated/opted-in); trusting it here is the same trust the +# internal decider applies. This is the ONLY path that authorizes a non-read +# action under this read-ACL skeleton. allow if { + input.user.is_active_admin +} + +# This policy models read-ACL only, so every cascade grant below is gated on a +# read action; any write/create/delete/admin falls through to the default-deny +# above (fail-closed). Empty/absent action == read. +is_read_action if { + not input.action +} + +is_read_action if { + input.action == "" +} + +is_read_action if { + input.action == "read" +} + +# Read allowed when no .zddc files anywhere in the chain AND no rule matches. +allow if { + is_read_action not input.policy_chain.has_any_file count(matched_levels) == 0 } -# Allow when the deepest matching level grants. +# Read allowed when the deepest matching level grants. allow if { + is_read_action count(matched_levels) > 0 deepest := max(matched_levels) level_grants(input.policy_chain.levels[deepest]) diff --git a/zddc/internal/policy/rego/access_federal.rego b/zddc/internal/policy/rego/access_federal.rego index 413e2e0..2a2786b 100644 --- a/zddc/internal/policy/rego/access_federal.rego +++ b/zddc/internal/policy/rego/access_federal.rego @@ -1,30 +1,26 @@ -# Federal-mode reference policy: parent-deny-is-absolute (NIST AC-6). +# Federal-mode reference SKELETON: parent-deny-is-absolute (NIST AC-6), +# read-ACL only. # -# This is a strict-least-privilege variant of access.rego. The two policies -# differ in exactly one rule, but the semantic difference is meaningful for -# federal evaluators: +# Like access.rego this models the read cascade ONLY and is NOT a complete +# authorization policy — it does not implement per-verb (write/create/delete/ +# admin), WORM, roles, inherit:false fences, or config-edit. It is therefore +# FAIL-CLOSED: every non-read action is denied. This variant deliberately has +# NO admin bypass either — under AC-6 least-privilege the default posture is +# deny, and an operator who needs a write path must add the per-verb (and, if +# desired, admin-escape) semantics themselves. As shipped it authorizes reads +# only. # -# access.rego (commercial, default): -# "Bottom-up walk; first explicit match wins; deny-first within a level. -# A leaf-level allow CAN override an ancestor's deny." -# Test: cascade_test.go "leaf allows user that parent denies → leaf wins". +# The ONE modelled difference from access.rego: any deny anywhere on the chain +# is absolute — a leaf-level allow does NOT override an ancestor's deny. +# Required by NIST AC-6: a central admin's root deny must be unbypassable by +# a tenant who controls a subtree's .zddc. +# access.rego (commercial): leaf allow CAN override an ancestor deny. +# access_federal.rego: ancestor deny is absolute. # -# access_federal.rego (federal): -# "Any deny anywhere along the chain is absolute. An allow only matters -# if no ancestor (or sibling level) has denied the same email. Leaf- -# level allows do NOT override ancestor denies." -# Required by NIST AC-6 (Least Privilege) default expectations: a -# central admin's deny at the root must be unbypassable by a tenant -# who controls a subtree's .zddc. -# -# Why ship two policies? The internal Go evaluator (in zddc/internal/zddc/ -# acl.go) implements only the commercial cascade — it's the rule the -# default deployment exercises. Federal customers running their own OPA -# with this file get the strict variant without any zddc-server code -# change. They can also write a hybrid policy (e.g. "deny is absolute -# only for emails matching some pattern; cascade rules for everyone -# else") since once they're hosting their own OPA, the constraint is -# whatever they write. +# The internal Go evaluator implements neither these federal semantics nor a +# tested mirror of this file; federal-mode is reachable only by running OPA +# with this policy and pointing ZDDC_OPA_URL at it. See federal_parity_test.go +# for the modelled read-cascade divergence fixtures. # # Input shape: identical to access.rego — see that file's docstring. # acl.permissions maps principal patterns to verb strings; an empty @@ -37,20 +33,37 @@ import future.keywords.in default allow := false -# Allow when no .zddc files exist anywhere AND no rule matches. +# Read-ACL only: every grant rule is gated on a read action; any write/ +# create/delete/admin falls through to the default-deny above (fail-closed). +# Empty/absent action == read. (No admin bypass in federal mode — see header.) +is_read_action if { + not input.action +} + +is_read_action if { + input.action == "" +} + +is_read_action if { + input.action == "read" +} + +# Read allowed when no .zddc files exist anywhere AND no rule matches. # Same default-allow case as commercial; preserves the empty-tree # behaviour. (zddc-server's --insecure check at startup makes this # unreachable in any non-deliberately-public deployment.) allow if { + is_read_action not input.policy_chain.has_any_file not any_deny_match not any_allow_match } -# Allow when files exist, no level (any depth) denies, and at least +# Read allowed when files exist, no level (any depth) denies, and at least # one level allows. The "any level" check is what makes parent denies # absolute — there is no "deepest match wins" rule here. allow if { + is_read_action input.policy_chain.has_any_file not any_deny_match any_allow_match diff --git a/zddc/internal/policy/rego_failclosed_test.go b/zddc/internal/policy/rego_failclosed_test.go new file mode 100644 index 0000000..e1f2dff --- /dev/null +++ b/zddc/internal/policy/rego_failclosed_test.go @@ -0,0 +1,98 @@ +package policy + +import ( + "context" + "testing" + + "codeberg.org/VARASYS/ZDDC/zddc/internal/zddc" + + "github.com/open-policy-agent/opa/rego" +) + +// TestReferenceRego_FailClosedOnWrites pins the security contract of the +// bundled reference Rego skeletons: they model READ-ACL only, so any non-read +// action must be DENIED even when the read-ACL would grant — and the commercial +// variant's only write-capable principal is an elevated admin. This is the +// behavior that, untested, previously let a verb-blind policy ship claiming to +// "mirror the internal decider exactly." See rego/access.rego. +func TestReferenceRego_FailClosedOnWrites(t *testing.T) { + ctx := context.Background() + + mkQuery := func(module, src string) rego.PreparedEvalQuery { + var pkg string + switch module { + case "access.rego": + pkg = "data.zddc.access.allow" + default: + pkg = "data.zddc.access_federal.allow" + } + q, err := rego.New(rego.Query(pkg), rego.Module(module, src)).PrepareForEval(ctx) + if err != nil { + t.Fatalf("compile %s: %v", module, err) + } + return q + } + stdQ := mkQuery("access.rego", ReferenceRego) + fedQ := mkQuery("access_federal.rego", FederalRego) + + // A chain that GRANTS full rwcd to alice — so any denial below is the + // action gate, not a missing ACL. + grant := zddc.PolicyChain{ + Levels: []zddc.ZddcFile{{ACL: zddc.ACLRules{Permissions: map[string]string{"*@example.com": "rwcd"}}}}, + HasAnyFile: true, + } + + evalAllow := func(q rego.PreparedEvalQuery, action string, admin bool) bool { + in := AllowInput{Path: "/p/", Action: action, PolicyChain: chainToSerializable(grant)} + in.User.Email = "alice@example.com" + in.User.IsActiveAdmin = admin + regoInput, err := canonicalInput(in) + if err != nil { + t.Fatalf("encode: %v", err) + } + rs, err := q.Eval(ctx, rego.EvalInput(regoInput)) + if err != nil { + t.Fatalf("eval: %v", err) + } + if len(rs) == 0 { + t.Fatalf("no result") + } + v, ok := rs[0].Expressions[0].Value.(bool) + if !ok { + t.Fatalf("result not bool: %v", rs[0].Expressions[0].Value) + } + return v + } + + cases := []struct { + name string + q rego.PreparedEvalQuery + action string + admin bool + wantAllow bool + }{ + // Commercial: reads granted, every write verb denied (fail-closed). + {"access read allowed", stdQ, ActionRead, false, true}, + {"access empty-action(read) allowed", stdQ, "", false, true}, + {"access write denied", stdQ, ActionWrite, false, false}, + {"access create denied", stdQ, ActionCreate, false, false}, + {"access delete denied", stdQ, ActionDelete, false, false}, + {"access admin-action denied", stdQ, ActionAdmin, false, false}, + // Commercial: an elevated admin is the one write-capable principal. + {"access write allowed for active admin", stdQ, ActionWrite, true, true}, + {"access delete allowed for active admin", stdQ, ActionDelete, true, true}, + + // Federal: reads granted, every write denied, and NO admin bypass. + {"federal read allowed", fedQ, ActionRead, false, true}, + {"federal write denied", fedQ, ActionWrite, false, false}, + {"federal admin-action denied", fedQ, ActionAdmin, false, false}, + {"federal write denied even for active admin", fedQ, ActionWrite, true, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := evalAllow(tc.q, tc.action, tc.admin); got != tc.wantAllow { + t.Errorf("allow=%v, want %v (action=%q active_admin=%v)", got, tc.wantAllow, tc.action, tc.admin) + } + }) + } +} diff --git a/zddc/internal/zddc/embedded_neutral_test.go b/zddc/internal/zddc/embedded_neutral_test.go new file mode 100644 index 0000000..7aca51b --- /dev/null +++ b/zddc/internal/zddc/embedded_neutral_test.go @@ -0,0 +1,45 @@ +package zddc + +import "testing" + +// TestEmbeddedDefaults_LosslessUnderSerialization pins the invariant that +// makes it safe for policy.SerializableChain to drop PolicyChain.Embedded on +// the wire (and for policy.InternalDecider.Allow to reconstruct a chain from +// AllowInput without it). +// +// The decision path consults chain.Embedded at two sites: +// - worm.go (WormZoneGrant): folds in a top-level chain.Embedded.Worm. +// - roles.go (lookupRoleMembers / pathRoles): reads chain.Embedded.Roles. +// +// Today both are no-ops only because the baked-in baseline (EmbeddedDefaults, +// the root of every cascade — internal/zddc/defaults/.zddc) declares no +// top-level `worm:` and no role members (WORM is declared via the `paths:` +// tree, which lands in chain.Levels, not chain.Embedded). If a future default +// adds either, those contributions would be SILENTLY ignored by an external +// OPA (it never receives Embedded) and by the InternalDecider over a +// serialized chain — an authz divergence with no error. This test fails loudly +// at that moment so the change is paired with serializing PolicyChain.Embedded +// (one field on SerializableChain) before it ships. +func TestEmbeddedDefaults_LosslessUnderSerialization(t *testing.T) { + e, err := EmbeddedDefaults() + if err != nil { + t.Fatalf("EmbeddedDefaults: %v", err) + } + + if e.Worm != nil { + t.Errorf("embedded baseline declares a top-level worm: %v — it is read at "+ + "decision time (WormZoneGrant) but dropped by policy.SerializableChain, "+ + "so external OPA and the reconstructed InternalDecider chain would "+ + "silently ignore it. Declare WORM via paths: (→ chain.Levels) or "+ + "serialize PolicyChain.Embedded.", e.Worm) + } + + for name, role := range e.Roles { + if len(role.Members) > 0 { + t.Errorf("embedded baseline role %q has members %v — read at decision "+ + "time (chain.Embedded.Roles in roles.go) but dropped by "+ + "policy.SerializableChain. Keep embedded roles member-less or "+ + "serialize PolicyChain.Embedded.", name, role.Members) + } + } +}