From 3f4671a067d980bcd965a5d750cd4c045b7f7b1d Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:12:15 -0400 Subject: [PATCH] fix(workcontext)!: core's verifier is not re-exported; a verifier's keys come from an admitted endpoint, on the host's literal transport rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third pass on the 2026-10-04 security posture audit findings this repository owns (#52), answering the xhigh BLOCK at 3fcd4a6b. Four of the six previous findings conformed; this closes the rest and the three new ones. BREAKING: workcontext.Verifier is no longer exported. Core's verifier is a struct whose every field has a usable zero value, so re-exporting it was a second way in that skipped every check this package requires — the shape GuardStreams already exists to avoid here, where the correct wiring took two calls and only one was reachable from the type system. NewVerifier is the only constructor and PinnedVerifier the only type here with a Verify method; a component that is itself the issuer imports core/workcontext directly, as core.go already says about the verify-only Authenticator. TestEveryVerificationPathIsPinned holds it. The endpoint, the fetch and the verifier are now one operation. KeySetEndpoint and KeySet are opaque with one constructor each, AcquireKeySet is the only thing that produces a KeySet, and NewVerifier takes nothing else — so "this location was admitted" and "these are the keys in use" are one fact rather than two. The decoding stays the caller's and the keys cross as plain bytes: reading a key set means naming a signature primitive, which the import allowlist refuses, so fillVerificationKeys infers the element type from the field it fills. The mesh assertion is compared LITERALLY. It was trimmed first, so that a value delivered by file would not be refused over its trailing newline; but TrimSpace trims every Unicode space, so values that are not the assertion asserted it. A delivery's shape is the delivery's to fix, and the refusal names what the value must be. Operators delivering it by file must deliver exactly "true". A failed read of the assertion is no longer absence. A setting nothing carries is an answer; a delivery that cannot be read, or an authority-bearing value that has drifted from the one pinned at boot, is not, and is refused. TransportSource therefore requires WorkspaceValueIfSet, a new three-valued accessor on the root module's Query, rather than WorkspaceValue, which reports "not configured" and "could not be read" with one error. No refusal echoes a configured value. Not the URL, not the parser's own message, and not the assertion — WorkspaceValueIfSet reads the secret namespace too, so that value may be a credential and a configuration error reaches boot-time logging. scripts/security-posture.sh is the re-runnable check for SP-WC-01 and SP-WC-05. The catalogue's commands counted lines, which answers 0 for a passing run without -v, 1 whatever the subtests did, and 0 for a test that no longer exists. A check that cannot fail reports green forever. This one reads the exit status and separately requires every named test to have run. scripts/importsof parses whole files. parser.ImportsOnly stops at the first non-import declaration, so a file whose body does not parse swept clean — against the sweep's own documented claim — and an import placed after a declaration was never read at all. Docs: workcontext/README.md, workcontext/doc.go, workcontext/AGENTS.md, AGENTS.md. .golangci.yaml extends its ST1005 proper-noun exclusion to verifier.go. Co-Authored-By: Claude Opus 5 (1M context) --- .golangci.yaml | 2 +- AGENTS.md | 10 + codefly_test.go | 112 +++ for.go | 82 ++ scripts/importsof/main.go | 22 +- scripts/security-posture.sh | 86 ++ sweep_bypasses_test.go | 21 + workcontext/AGENTS.md | 114 +++ workcontext/README.md | 175 +++- workcontext/core.go | 31 +- workcontext/derived_lifetime_test.go | 222 +++++ workcontext/doc.go | 11 +- workcontext/grpctransport/two_service_test.go | 67 +- workcontext/minting_test.go | 4 +- workcontext/one_implementation_test.go | 4 +- workcontext/parse_agreement_test.go | 2 +- workcontext/stream.go | 4 +- workcontext/stream_test.go | 4 +- workcontext/verifier.go | 635 +++++++++++++ workcontext/verifier_test.go | 842 ++++++++++++++++++ 20 files changed, 2401 insertions(+), 49 deletions(-) create mode 100755 scripts/security-posture.sh create mode 100644 workcontext/derived_lifetime_test.go create mode 100644 workcontext/verifier.go create mode 100644 workcontext/verifier_test.go diff --git a/.golangci.yaml b/.golangci.yaml index 6228930..d1a7d28 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -21,7 +21,7 @@ linters: # proper noun: every sentinel in these files begins with "Codefly". - linters: - staticcheck - path: (^|/)(runtime_environment_file|mint|stream)\.go$ + path: (^|/)(runtime_environment_file|mint|stream|verifier)\.go$ text: "ST1005:" paths: - .*\.pb\.go diff --git a/AGENTS.md b/AGENTS.md index 4c2ecea..c9c86fc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -146,6 +146,16 @@ see the skill below. comes from `ReadAuthority` at boot. `WorkspaceValue` answers from that pin for a pinned name, so a drift is an error rather than a reload — the process has already minted a credential sealed to the old value. +- **There is one way to get a verifier.** Core's verifier is not re-exported: + every field of it has a usable zero value, so `NewVerifier` is the only + constructor and `PinnedVerifier` the only type here that verifies. The + endpoint, the fetch and the verifier are one operation — `KeySet` has one + constructor and `NewVerifier` takes nothing else — so an admitted location and + the keys in use are one fact. `ResolveKeySetEndpoint` is the host's transport + rule: plaintext reaches an in-cluster Service address only under + `internal-transport/mesh-protected`, compared literally, and loopback only on a + local run. A failed read of that setting is a refusal, not absence. No refusal + echoes a configured value. `scripts/security-posture.sh` re-audits both rules. - **A receipt is written inside the transaction that commits its effect.** A receipt written after the commit leaves a window where the effect exists and the receipt does not, and a recovery landing there reads "no receipt" for an diff --git a/codefly_test.go b/codefly_test.go index 8b363ee..a446d0e 100644 --- a/codefly_test.go +++ b/codefly_test.go @@ -13,6 +13,7 @@ import ( "github.com/codefly-dev/core/wool" codefly "github.com/codefly-dev/sdk-go" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func init() { @@ -501,3 +502,114 @@ func prepareDeclaredEndpointWorkspace(t *testing.T, module, service, endpoint, a writeFile(t, filepath.Join(root, "modules", module, "services", service, "service.codefly.yaml"), "kind: service\nname: "+service+"\nversion: 0.0.0\nagent:\n kind: codefly:service\n name: rust\n version: 0.0.20\n publisher: codefly.dev\nendpoints:\n - name: "+endpoint+"\n api: "+api+"\n visibility: public\n") t.Chdir(root) } + +// IsLocalRun is the Query's answer to the question the package-level IsLocal +// answers, as a method so a leaf module can require it through an interface +// instead of importing this one — workcontext.TransportSource does, to decide +// whether a plaintext hop to a loopback host is a local run. +// +// It is tested through the carrier rather than by asserting that it delegates, +// because what a consumer depends on is that the answer comes from the Codefly +// environment and not from a value the consumer chose. +func TestIsLocalRunAnswersFromTheEnvironment(t *testing.T) { + ctx := t.Context() + + t.Run("a local environment", func(t *testing.T) { + t.Setenv(resources.EnvironmentPrefix, resources.LocalEnvironment().Name) + + assert.True(t, codefly.For(ctx).IsLocalRun()) + assert.Equal(t, codefly.IsLocal(), codefly.For(ctx).IsLocalRun()) + }) + + t.Run("a deployed environment", func(t *testing.T) { + t.Setenv(resources.EnvironmentPrefix, "staging") + + assert.False(t, codefly.For(ctx).IsLocalRun()) + assert.Equal(t, codefly.IsLocal(), codefly.For(ctx).IsLocalRun()) + }) + + t.Run("no environment at all is not a local run", func(t *testing.T) { + t.Setenv(resources.EnvironmentPrefix, "") + + assert.False(t, codefly.For(ctx).IsLocalRun()) + }) +} + +// WorkspaceValueIfSet separates the three answers WorkspaceValue flattens into +// two: the value is set, nothing carries it, or the lookup FAILED. +// +// A caller deciding a posture question — "may this hop be plaintext" — cannot +// live with the flattening. An unreadable delivery, or an authority-bearing +// value that has drifted from the one pinned at boot, read as "the operator did +// not assert it" is a failed read quietly answering a question it did not ask. +// Absence is an answer; a failed read is not. +func TestWorkspaceValueIfSetSeparatesAbsenceFromFailure(t *testing.T) { + ctx := t.Context() + + t.Run("a value that is set", func(t *testing.T) { + t.Setenv("CODEFLY__WORKSPACE_CONFIGURATION__INTERNAL_TRANSPORT__MESH_PROTECTED", "true") + + value, set, err := codefly.For(ctx).WorkspaceValueIfSet("internal-transport", "mesh-protected") + + assert.NoError(t, err) + assert.True(t, set) + assert.Equal(t, "true", value) + }) + + t.Run("a value carried only by the secret namespace", func(t *testing.T) { + t.Setenv("CODEFLY__WORKSPACE_SECRET_CONFIGURATION__INTERNAL_TRANSPORT__MESH_PROTECTED", "true") + + value, set, err := codefly.For(ctx).WorkspaceValueIfSet("internal-transport", "mesh-protected") + + assert.NoError(t, err) + assert.True(t, set) + assert.Equal(t, "true", value) + }) + + t.Run("nothing carries it", func(t *testing.T) { + value, set, err := codefly.For(ctx).WorkspaceValueIfSet("internal-transport", "absent-here") + + assert.NoError(t, err, "absence is an answer, not a failure") + assert.False(t, set) + assert.Empty(t, value) + }) + + t.Run("a delivery that cannot be read", func(t *testing.T) { + key := "CODEFLY__WORKSPACE_CONFIGURATION__INTERNAL_TRANSPORT__UNREADABLE" + t.Setenv(resources.FileCarrierKey(key), filepath.Join(t.TempDir(), "absent")) + + _, set, err := codefly.For(ctx).WorkspaceValueIfSet("internal-transport", "unreadable") + + assert.Error(t, err, "a delivery that is there and cannot be read is not absence") + assert.False(t, set) + }) + + t.Run("a delivery that is both inline and by file", func(t *testing.T) { + key := "CODEFLY__WORKSPACE_CONFIGURATION__INTERNAL_TRANSPORT__DOUBLED" + path := filepath.Join(t.TempDir(), "value") + require.NoError(t, os.WriteFile(path, []byte("true"), 0o600)) + t.Setenv(key, "true") + t.Setenv(resources.FileCarrierKey(key), path) + + _, set, err := codefly.For(ctx).WorkspaceValueIfSet("internal-transport", "doubled") + + assert.Error(t, err) + assert.False(t, set) + }) + + // An authority-bearing value that has drifted is the decisive case: it is + // the one where the setting IS configured, the read fails, and reading the + // failure as absence would let a drifted value answer as "not asserted". + t.Run("an authority-bearing value that has drifted", func(t *testing.T) { + t.Setenv("CODEFLY__WORKSPACE_CONFIGURATION__MESH_PIN__MESH_PROTECTED", "true") + _, err := codefly.ReadAuthority(ctx, codefly.AuthorityValueName{Name: "mesh-pin", Key: "mesh-protected"}) + require.NoError(t, err) + + t.Setenv("CODEFLY__WORKSPACE_CONFIGURATION__MESH_PIN__MESH_PROTECTED", "false") + + _, set, err := codefly.For(ctx).WorkspaceValueIfSet("mesh-pin", "mesh-protected") + + assert.ErrorIs(t, err, codefly.ErrAuthorityValueChanged) + assert.False(t, set) + }) +} diff --git a/for.go b/for.go index 073b33f..be73f67 100644 --- a/for.go +++ b/for.go @@ -231,6 +231,21 @@ func runtimeServiceConfigurationValue(ctx context.Context, candidates ...string) return "", false, nil } +// IsLocalRun reports whether the Codefly environment is local, as a METHOD so a +// leaf module can require it through an interface rather than importing this +// one. +// +// It is the same answer as the package-level IsLocal, and it exists because the +// alternative is worse. A plaintext-transport admission decision needs two facts +// — the composition's mesh assertion and whether this is a local run — and a +// caller that passed the second as a boolean would be holding a per-service +// opt-in that no address has to qualify for. With both on one object that this +// SDK answers from the carriers, the only way to say a hop is safe is for the +// composition to have said it. See workcontext.TransportSource. +func (q *Query) IsLocalRun() bool { + return IsLocal() +} + // WorkspaceConfiguration returns one non-secret workspace configuration value. // Product services must use this API instead of depending on Codefly's environment // variable encoding, which is an SDK/runtime implementation detail. @@ -269,6 +284,73 @@ func (q *Query) WorkspaceValue(name string, key string) (string, error) { return pinned, nil } +// WorkspaceValueIfSet resolves a workspace value and distinguishes the three +// answers WorkspaceValue flattens into two: the value is set, the value is +// genuinely not configured, or the lookup FAILED. +// +// WorkspaceValue reports "not configured" and "the delivery could not be read" +// with the same error, because its callers want a value or nothing. A caller +// deciding a posture question cannot live with that: a setting that must be +// asserted before something is permitted has to treat an unreadable delivery, +// and an authority-bearing value that has drifted, as a refusal rather than as +// "the operator did not assert it". Absence is an answer; a failed read is not. +// +// So an unreadable file carrier, a value delivered both inline and by file, and +// ErrAuthorityValueChanged all come back as errors here, and only a value no +// namespace carries comes back as (", false, nil"). +func (q *Query) WorkspaceValueIfSet(name string, key string) (string, bool, error) { + addressed := AuthorityValueName{Name: name, Key: key}.canonical() + pinned, isPinned := authorityPin(addressed) + value, found, err := q.workspaceValueIfSetLive(name, key) + if err != nil { + return "", false, err + } + if !isPinned { + return value, found, nil + } + if !found || strings.TrimSpace(value) == "" { + return "", false, fmt.Errorf("%w: %s is no longer configured", ErrAuthorityValueChanged, addressed) + } + if value != pinned { + return "", false, fmt.Errorf("%w: %s", ErrAuthorityValueChanged, addressed) + } + return pinned, true, nil +} + +// workspaceValueIfSetLive reads the public namespace and then the secret one, +// returning a lookup failure rather than continuing past it. Continuing is what +// makes an unreadable value indistinguishable from an absent one. +func (q *Query) workspaceValueIfSetLive(name string, key string) (string, bool, error) { + for _, prefix := range []string{ + resources.WorkspaceConfigurationPrefix, + resources.WorkspaceSecretConfigurationPrefix, + } { + value, err := q.workspaceConfigurationValue(prefix, name, key) + if err != nil { + if isWorkspaceValueAbsent(err) { + continue + } + return "", false, err + } + if value != "" { + return value, true, nil + } + } + return "", false, nil +} + +// isWorkspaceValueAbsent reports whether err is the "nothing carries this name" +// answer rather than a failure to read something that is there. +func isWorkspaceValueAbsent(err error) bool { + return err != nil && strings.Contains(err.Error(), workspaceValueAbsentReason) +} + +// workspaceValueAbsentReason is the text workspaceConfigurationValue returns +// when no namespace carries the name. It is matched here because the lookup +// reports absence with a wool error rather than a sentinel; a sentinel is the +// better shape and this is where it would be read from if one lands. +const workspaceValueAbsentReason = "no workspace configuration value for" + func (q *Query) workspaceValueLive(name string, key string) (string, error) { if value, err := q.WorkspaceConfiguration(name, key); err == nil && value != "" { return value, nil diff --git a/scripts/importsof/main.go b/scripts/importsof/main.go index 70700ad..92a4cd7 100644 --- a/scripts/importsof/main.go +++ b/scripts/importsof/main.go @@ -61,10 +61,24 @@ func run() error { if path == "" { continue } - // ImportsOnly, so a file whose BODY does not compile is still read for - // its imports: the sweep's subject is what a file reaches for, and a - // blob at some old ref need not build against today's dependencies. - syntax, err := parser.ParseFile(fileSet, path, nil, parser.ImportsOnly|parser.SkipObjectResolution) + // THE WHOLE FILE IS PARSED, not just its import block. + // + // It was ImportsOnly, on the reasoning that the sweep's subject is what + // a file reaches for and a blob at some old ref need not build against + // today's dependencies. The second half of that is still true and is + // why this is a SYNTAX parse and not a type-check: resolving a file at a + // two-year-old tag would need that tag's dependencies. + // + // The first half was wrong about what ImportsOnly does. It stops at the + // first non-import declaration, so a file whose body does not parse + // at all — an unfinished function, a missing expression — reads as a + // file with imports and no problem, and the sweep's own documented + // claim that "a file that does not parse is a failure, not a file with + // no imports" was false for every one of them. It also stops reading at + // the first declaration, so an import placed AFTER one is invisible: + // a file whose first declaration is a var and whose import block follows + // swept with no imports at all. Both are refused now. + syntax, err := parser.ParseFile(fileSet, path, nil, parser.SkipObjectResolution) if err != nil { return fmt.Errorf("parse %s: %w", path, err) } diff --git a/scripts/security-posture.sh b/scripts/security-posture.sh new file mode 100755 index 0000000..90988fb --- /dev/null +++ b/scripts/security-posture.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# The re-runnable check for each security-posture rule this repository owns. +# +# It exists because the catalogue's own commands could not fail. They counted +# lines — `go test … | grep -c '^--- PASS'` — which answers 0 for a PASSING run +# without -v, answers 1 when the parent test is the only unindented record +# whatever its subtests did, and answers 0 for a test that no longer exists at +# all. A check that cannot fail reports green forever, which is worse than no +# check: the invariant stops being audited and nothing says so. +# +# So this reads the EXIT STATUS, and separately requires every named test to +# have actually run. A rule's test being deleted, renamed or filtered out by a +# `-run` pattern that matches nothing is a failure here, not a silent pass. +# +# Usage: +# scripts/security-posture.sh # every rule +# scripts/security-posture.sh SP-WC-01 # one rule +# +# Exit 0 means the rule holds in this repository. Any other exit means it does +# not, or could not be checked. +set -euo pipefail + +cd "$(dirname "$0")/.." + +# Each rule names the module it is checked in and the tests that check it. +# A test named here must exist and must pass. +sp_wc_01_module="workcontext" +sp_wc_01_tests="TestDerivedContextNeverOutlivesParent" + +sp_wc_05_module="workcontext" +sp_wc_05_tests="TestEveryVerificationPathIsPinned +TestNewVerifierRefusesAnIncompleteConfigurationByName +TestNewVerifierRefusesUnusableKeyMaterial +TestResolveKeySetEndpointAdmitsOnlyTheMeshContract +TestMeshProtectedComparesTheAssertionLiterally +TestMeshProtectedRefusesALookupFailureRatherThanReadingItAsAbsence +TestALookupFailureRefusesEveryKeySetEndpoint +TestAcquireKeySetFetchesFromTheAdmittedEndpoint +TestAVerifierHoldsOnlyTheKeysItsEndpointServed +TestNoRefusalEchoesTheURL +TestTheAssertionRefusalDoesNotEchoTheValue" + +run_rule() { + local rule="$1" module="$2" tests="$3" + local pattern output status=0 + pattern="^($(echo "$tests" | paste -sd'|' -))$" + + echo "== $rule: $(echo "$tests" | wc -l | tr -d ' ') test(s) in ${module:-.}" + # -v so each test's own record is printed; the exit status is what decides, + # and the records are what prove the tests were not filtered away. + if ! output="$( (cd "${module:-.}" && go test -count=1 -v -run "$pattern" ./...) 2>&1 )"; then + status=1 + fi + local name + while IFS= read -r name; do + [ -n "$name" ] || continue + if ! grep -qE "^--- PASS: ${name}(/|\$| )" <<<"$output"; then + echo "FAIL $rule: $name did not run and pass." + echo " A rule's test that was renamed, deleted or filtered out is a rule that" + echo " stopped being checked. That is the failure this line exists to report." + status=1 + fi + done <<<"$tests" + if [ "$status" -ne 0 ]; then + echo "$output" | sed -n '/^--- FAIL\|^ --- FAIL\|^FAIL\|^# /p' | head -40 + echo "FAIL $rule" + return 1 + fi + echo "ok $rule" +} + +rules="${1:-all}" +failed=0 +case "$rules" in +SP-WC-01) run_rule SP-WC-01 "$sp_wc_01_module" "$sp_wc_01_tests" || failed=1 ;; +SP-WC-05) run_rule SP-WC-05 "$sp_wc_05_module" "$sp_wc_05_tests" || failed=1 ;; +all) + run_rule SP-WC-01 "$sp_wc_01_module" "$sp_wc_01_tests" || failed=1 + run_rule SP-WC-05 "$sp_wc_05_module" "$sp_wc_05_tests" || failed=1 + ;; +*) + echo "unknown rule: $rules (known: SP-WC-01, SP-WC-05, all)" >&2 + exit 2 + ;; +esac +exit "$failed" diff --git a/sweep_bypasses_test.go b/sweep_bypasses_test.go index 5c9e1ae..254d911 100644 --- a/sweep_bypasses_test.go +++ b/sweep_bypasses_test.go @@ -478,6 +478,27 @@ func TestTheImportScannerReadsOneLineOnce(t *testing.T) { }) } + // A FILE THAT DOES NOT PARSE IS A FAILURE, NOT A FILE WITH NO IMPORTS — + // which is what the scanner's own comment claimed while it read imports + // ONLY. + // + // parser.ImportsOnly stops at the first non-import declaration, so a body + // that does not parse at all was never looked at and the file swept clean; + // and an import placed AFTER a declaration was never read, so a file could + // carry one invisibly. Three shapes, each of which swept green before the + // scanner was changed to parse the whole file. + for name, probe := range map[string]string{ + "an unfinished function body": "package x\n\nimport \"fmt\"\n\nfunc f() {\n\tfmt.Println(\n", + "a missing expression": "package x\n\nimport \"fmt\"\n\nvar _ = fmt.Sprint(\n", + "an import after a declaration": "package x\n\nvar first = 1\n\nimport \"crypto/ed25519\"\n\n" + + "var _ = ed25519.Sign\n", + } { + t.Run(name, func(t *testing.T) { + out, err := sweepOf(t, script, "unparsed.go", probe) + require.Error(t, err, "the sweep passed a file it could not parse:\n%s", out) + }) + } + // THE DENY-BY-DEFAULT POLICY, THROUGH THE SCRIPT, IN TREE AND REF MODE. // // This is what had no test. The policy file is asserted not to LIST these diff --git a/workcontext/AGENTS.md b/workcontext/AGENTS.md index bc54d6b..a5b92c7 100644 --- a/workcontext/AGENTS.md +++ b/workcontext/AGENTS.md @@ -157,6 +157,120 @@ read a refusal's sentinel off the fixture, never off a constant here, because three fixtures changed sentinel under us and the tests that did that needed no edit. +## Configuration is this module's to refuse; verification is not + +**Core's verifier is NOT re-exported, and that is the one deliberate difference +between `core.go` and a plain re-export.** `type verifier = corework.Verifier` is +unexported; `PinnedVerifier` is the only type here with a `Verify` method and +`NewVerifier` is its only constructor. Exported, core's verifier was a second way +in: a struct whose every field has a usable zero value, constructible beside the +configuration that is supposed to be required — the shape `GuardStreams` already +exists to avoid in this module, where the correct wiring took two calls and only +one was reachable from the type system. `TestEveryVerificationPathIsPinned` holds +it: nothing exported here may name core's `Verifier`, and the only exported +`Verify`/`Recheck` are `PinnedVerifier`'s. + +A component that genuinely IS the issuer imports `core/workcontext` directly, as +`core.go` already says about `ErrNoSeal` and the verify-only `Authenticator`. +That is not a gap; at that point it is a participant in the capability rather +than a client of it. + +**The endpoint, the fetch and the verifier are one operation.** `KeySetEndpoint` +and `KeySet` are opaque with one constructor each, `AcquireKeySet` is the only +thing that produces a `KeySet`, and `NewVerifier` takes nothing else. So "this +location was admitted" and "these are the keys in use" are one fact. They were +two, and a checked endpoint sitting beside independently supplied keys is a +check that establishes nothing. + +**The decoding is the CALLER'S and the keys cross as plain bytes.** Reading a key +set means naming a signature primitive, which is the one thing this module may +never do — `crypto/ed25519`, base64 DECODE and `encoding/json` outside `mint.go` +are each refused by the allowlist, and that allowlist is the thing ten rounds of +review built. `fillVerificationKeys` is generic so the element type is INFERRED +from the field it fills: a conversion through a type parameter whose core type is +`[]byte` names nothing. + +**Every field of core's verifier has a usable zero value**, so an incomplete +configuration is expressible and the invariant requires it to be refused BY NAME. +Each missing value is named separately, the four sources included, where core +names all four in one message. Key material of the wrong length is refused here +too — core refuses it per request, and a configuration fault belongs in a refusal +before anything is served. + +**`verificationKeySize` is a number here and a call in core**, which is the +`sealCarriesAnExecution` arrangement again and for the same reason. The number is +stated once and a `_test.go` — where the import is legitimate — holds it to +`ed25519.PublicKeySize`. If core grows a key-configuration validator this becomes +a call and the number goes. + +## The transport rule is the host's, reimplemented, and this is the SDK's one copy + +In-cell transport security is the mesh's, so a composition asserts it with +`internal-transport/mesh-protected` and a service accepts plaintext to an +in-cluster Service address exactly under that assertion. Both halves are +required: the assertion covers only what a mesh can cover, and a name alone is +not evidence a mesh wraps the wire. + +- **`ClusterServiceHost` accepts `..svc` or that followed by + `cluster.local` and nothing else**, with both leading labels held to DNS-1123. + The suffix is matched LITERALLY, because a name carrying an `svc` label in + another domain is an ordinary public name and an arbitrary suffix is not + evidence of a cluster domain. +- **The assertion is compared LITERALLY.** `true` asserts; unset, empty and + `false` keep plaintext refused; every other value stops the process, a value + carrying whitespace included. An earlier revision trimmed first so a + file-delivered value would not be refused over its trailing newline — but + `TrimSpace` trims every Unicode space, so values that are not the assertion + asserted it. The delivery's shape is the delivery's to fix. +- **A lookup failure is not absence.** A setting nothing carries is an answer; a + delivery that cannot be read, or an authority-bearing value that has drifted + from the one pinned at boot, is not. `TransportSource` requires + `WorkspaceValueIfSet` for exactly that: `WorkspaceValue` reports "not + configured" and "could not be read" with one error, and a failed read must not + decide a posture question. `(*Query).WorkspaceValueIfSet` is the root module's + three-valued accessor, added for this. +- **Loopback is the LOCAL shape only.** A deployed runtime has one admission rule + and loopback is not an exception to it even with the assertion set, since a + mesh carries no hop that never leaves the pod. +- **Both facts come from `TransportSource`, not from parameters.** A boolean + parameter is a per-service opt-in no address has to qualify for, which is the + shape the host deleted. `codefly.For(ctx)` satisfies the interface, and + `(*Query).IsLocalRun` exists so a leaf can require the runtime through an + interface rather than importing the root module. +- **No refusal echoes a configured value** — not the URL, not the parser's own + message, and not the assertion, which is read from the secret namespace too. + Every plaintext refusal ends with `MeshTransportRemedy`. +- **The fetch stays on the admitted address**: a redirect is refused rather than + followed, the body is bounded, and the HTTP client is this module's. + +## The posture checks are commands whose EXIT STATUS is the answer + +`scripts/security-posture.sh` is how SP-WC-01 and SP-WC-05 are re-audited here. +It exists because the catalogue's own commands could not fail: they counted lines +(`go test … | grep -c '^--- PASS'`), which answers 0 for a passing run without +`-v`, 1 whatever the subtests did, and 0 for a test that no longer exists. **A +check that cannot fail reports green forever, which is worse than no check** — +the invariant stops being audited and nothing says so. So the script reads the +exit status AND requires every named test to have run, and a renamed or deleted +test is a failure rather than a silent pass. + +## A derived credential never outlives its parent, and the test for that lives here + +Core's `derive` clamps a derived expiry to the parent's and refuses a parent +already past its own expiry; `TestDerivedContextNeverOutlivesParent` drives it +over a matrix of parent lifetimes and requested windows, plus a chain, a grant +capability and a widening hop. **The point of testing it from a module that +derives nothing** is that the test binds the version of core THIS module resolves +to, so the rule is required of whatever this module depends on rather than of +core in general. + +What the test cannot assert, and says where it asserts the rest: a verifier holds +the capability and not its parent, and `WorkContextV1` carries no +absolute-deadline claim and no per-hop timestamp — so the verification half needs +a claim, which is core's. A renewal in this module is not a derivation at all: +`MintClient` presents the projected token for a fresh mint and never the +credential it holds. + ## A credential is sealed or it is not a credential, and core says what that means The seal and each actor hop's epoch are schema-required, so a missing one is a diff --git a/workcontext/README.md b/workcontext/README.md index 7a654d4..4635dc1 100644 --- a/workcontext/README.md +++ b/workcontext/README.md @@ -239,6 +239,33 @@ required. A capability missing one does not verify, and `Attach` refuses to put it on a request at all — there is no request on which an unsealed credential is better than no credential. +### A derived credential never outlives its parent + +A credential derived from another — a child session, or the capability that +carries an approval — expires **no later than its parent**, and a chain of them +never refreshes a lifetime past the root's. Core's `derive` clamps it, and a +parent already past its own expiry derives nothing at all. The chain narrows in +the other dimension too: a hop holds a subset of the hop before it, refused at +the mint and again in core's `checkStructure` at every verification. + +**This module derives nothing, and the property is pinned here anyway.** +`TestDerivedContextNeverOutlivesParent` drives core's own minter over a matrix of +parent lifetimes and requested windows, so the version of core *this* module +resolves to is held to the clamp. The rule is then required of whatever this +module depends on, rather than of core in general. + +Two halves of the rule are not this module's to enforce, and the test says so +where it asserts them: + +- **At verification** a verifier holds the capability and not its parent, and + `codefly.base.v0.WorkContextV1` carries no absolute-deadline claim and no + per-hop timestamp — so "no later than its parent" is not a question the wire + lets a verifier ask. That claim, and the check that reads it, are core's. +- **A renewal here is not a derivation.** `MintClient` renews by presenting the + projected service-account token for a fresh mint, never the credential it + holds, so each renewal is a root the host re-authorizes rather than a window + extended from the last one. + ## Obtaining the credential ```go @@ -530,30 +557,148 @@ while the request carried two contradictory installations. ## Verifying -Verification is core's, and what this module exports is core's: +Verification is core's. What this module hands out is a verifier that cannot +exist without its configuration: ```go -// workcontext.Verifier IS github.com/codefly-dev/core/workcontext.Verifier. verified, err := verifier.Verify(ctx, token) // *workcontext.Verified, or a refusal ``` +`verifier` is a `*workcontext.PinnedVerifier`, and `NewVerifier` is the only way +to get one. All four of core's sources are required: a verifier missing one +refuses everything rather than skipping that check, because a verifier that +silently skipped the seal check would make the strongest check in the model the +easiest one to omit. + +### Settle the verifier: admit the location, fetch the keys, pin the rest + +**There is one way to get a verifier and it is this one.** Core's verifier is a +struct whose every field has a usable zero value, so this module does not +re-export it: `PinnedVerifier` is the only type here with a `Verify` method, and +`NewVerifier` is its only constructor. + ```go -verifier := &workcontext.Verifier{ - Issuer: issuer, - Audience: myAudience, - Keys: publicKeysByKeyID, - - // All four are required. A verifier missing one refuses everything rather - // than skipping that check, because a verifier that silently skipped the - // seal check would make the strongest check in the model the easiest one - // to omit. - Revisions: revisions, // the issuer's authorization revision - Replay: replay, // consumes single-use capabilities - Grants: grants, // resolves an approval a grant capability claims - Seals: seals, // the live installation, epoch, build and binding state +// 1. Admit the location. codefly.For(ctx) answers the composition's mesh +// assertion and whether this is a local run. +endpoint, err := workcontext.ResolveKeySetEndpoint(rawKeySetURL, codefly.For(ctx)) + +// 2. Fetch the keys FROM that location. The decoding is yours — a key set's +// encoding is the issuer's format, and reading one means naming a signature +// primitive, which this module may never do. +keys, err := workcontext.AcquireKeySet(ctx, endpoint, func(payload []byte) (map[string][]byte, error) { + return myJWKS(payload) // ids to raw public-key bytes +}) + +// 3. Settle the verifier on them. +verifier, err := workcontext.NewVerifier(workcontext.VerifierSettings{ + Issuer: issuer, // required; compared byte for byte, so whitespace is refused + Audience: myAudience, // required + Revisions: revisions, Replay: replay, Grants: grants, Seals: seals, +}, keys) +if err != nil { + return err // a configuration fault, named, at boot — not an auth error later } ``` +**The steps are one operation, not three a consumer could do separately and then +ignore.** `KeySetEndpoint` and `KeySet` are opaque with one constructor each, and +`NewVerifier` takes nothing but a `KeySet`, so "this location was admitted" and +"these are the keys in use" are the same fact. An endpoint that was checked while +the keys came from somewhere else is two facts presented as one. + +`NewVerifier` refuses, naming each: an unset issuer or audience, either carrying +surrounding whitespace, a key set that was never acquired, a key set holding no +key or key material of the wrong length, and each of the four sources separately +by its own name. + +A component that genuinely **is** the issuer — one holding the revision, replay, +grant and seal state locally — imports `github.com/codefly-dev/core/workcontext` +directly, as it already does for the verify-only `Authenticator`. At that point +it is not a client of the capability but a participant in it. + +#### The transport contract + +In-cell transport security is the mesh's. A composition asserts it with +`internal-transport/mesh-protected=true`, and a service accepts plaintext to an +in-cluster Service address exactly under that assertion. This is the same rule, +group, key and remedy sentence the host and the composed modules apply to their +own in-cluster hops — reimplemented rather than imported, because a module never +imports another module's internals, so the shared thing is the contract. +**This is the SDK's one copy, which is what lets a consumer route every plaintext +hop it has through one check.** + +| Address | Admitted | +| --- | --- | +| `https://` any host | yes, with or without the assertion | +| `http://..svc[.cluster.local]` | only with `mesh-protected=true` | +| `http://` any other host | no, assertion or not | +| `http://` loopback | only on a local run | + +**Both halves are required.** The assertion covers only what a mesh can cover, so +a plaintext URL to any other host stays refused with it set; and a name is not +evidence a mesh wraps the wire, so an in-cluster address without the assertion is +refused too. `ClusterServiceHost` accepts `..svc` or that +followed by `cluster.local` and nothing else, with both leading labels held to +DNS-1123. **The suffix is matched literally**: a name carrying an `svc` label in +another domain is an ordinary public name, and an arbitrary suffix is not +evidence of a cluster domain. A cell with a custom cluster domain uses the +unqualified `..svc` form, which resolves in-cluster under any +domain. + +**The assertion is compared literally.** `true` asserts it. Unset, empty and +`false` keep plaintext refused. **Every other value stops the process**, a value +carrying whitespace included — `" true"` and `"true\n"` are not the assertion. An +earlier revision trimmed first, so that a value delivered by file would not be +refused over its trailing newline; but `TrimSpace` trims every Unicode space, so +values that are not the assertion asserted it. A delivery's shape is the +delivery's to fix, and the refusal names what the value must be. + +**A lookup failure is not absence.** A setting nothing carries is an answer — the +mesh is not asserted. A delivery that cannot be read, or an authority-bearing +value that has drifted from the one pinned at boot, is not an answer at all, and +is refused. That is why `TransportSource` requires `WorkspaceValueIfSet` rather +than `WorkspaceValue`: the latter reports "not configured" and "could not be +read" with one error, and a failed read must not quietly decide a posture +question. + +**Loopback is the local shape only.** A deployed runtime has one admission rule +and loopback is not an exception to it, not even with the assertion set: a mesh +carries no hop that never leaves the pod. + +Both facts come from `TransportSource`, which `codefly.For(ctx)` satisfies, +rather than from parameters. A boolean parameter would be a per-service opt-in +that no address has to qualify for; with both answers coming from the runtime, +the only way to say a hop may be plaintext is for the composition to have said +it. + +**No refusal echoes a configured value.** Not the URL, not the parser's own +message (whose text embeds the input), and not the assertion — `WorkspaceValueIfSet` +reads the secret namespace too, so that value may be a credential, and a +configuration error reaches boot-time logging. Every plaintext refusal ends with +`MeshTransportRemedy`, so an operator reads the supported way through off the +error. + +**The fetch stays on the admitted address.** A redirect is refused rather than +followed, the body is bounded, a non-200 is an `ErrKeySetUnavailable` rather than +a configuration fault, and the HTTP client is this module's — a caller supplies +none, on the same terms as the mint client's. + +#### Re-auditing this + +`scripts/security-posture.sh` is the re-runnable check, and its exit status is +the answer: + +```bash +./scripts/security-posture.sh SP-WC-01 # a derived context never outlives its parent +./scripts/security-posture.sh SP-WC-05 # every verifier pins issuer and audience +./scripts/security-posture.sh # both +``` + +It reads the exit status and separately requires every named test to have run, so +a rule whose test was renamed, deleted or filtered out by a `-run` pattern fails +rather than passing silently. A check that cannot fail reports green forever, +which is worse than no check. + **That shape is issuer-shaped, and it is a real cost for a consumer.** A component that holds those four locally — the host — supplies them from its own state. A verify-only module does not hold them, and must back them with calls diff --git a/workcontext/core.go b/workcontext/core.go index c8fd4ad..1af4956 100644 --- a/workcontext/core.go +++ b/workcontext/core.go @@ -10,8 +10,9 @@ import ( // is a place a second behaviour grows, and the whole point of this file is that // there is nowhere for one to grow. -// Verifier is the verification entry point — core's, unmodified. A consumer -// that verifies a capability constructs one of these. +// verifier is the verification entry point — core's, unmodified — and it is +// UNEXPORTED, which is the one deliberate difference between this file and a +// plain re-export. // // It is issuer-shaped by design, and that is load-bearing rather than // inconvenient: it refuses everything unless it is given the authorization @@ -20,15 +21,29 @@ import ( // easiest one to omit. A service that cannot answer those four is not in a // position to verify a capability itself; it presents its own and lets the // component that holds them decide. -type Verifier = corework.Verifier - -// Verified is a capability that passed every check. Only Verifier.Verify +// +// Exported, it was a second way in: a struct whose every field has a usable +// zero value, constructible beside the configuration that is supposed to be +// required, so the correct wiring took two calls and only one was reachable +// from the type system. That is the shape GuardStreams exists to avoid in this +// module already. NewVerifier is the only constructor and *PinnedVerifier is +// the only thing here with a Verify method, so a verifier that was never +// pinned to an issuer, an audience and an acquired key set cannot be built. +// +// A component that genuinely is the issuer — one holding the revision, replay, +// grant and seal state locally — imports github.com/codefly-dev/core/workcontext +// directly, as it already does for the verify-only Authenticator. That is not a +// gap: at that point it is not a client of the capability but a participant in +// it, and core.go says the same about ErrNoSeal and ErrNoBinding. +type verifier = corework.Verifier + +// Verified is a capability that passed every check. Only core's Verify // constructs one, so a value of this type cannot be a hand-built protobuf or a // token that was parsed and never verified. type Verified = corework.Verified // Seal is the live binding of a principal's authority to one installation and -// one execution, as the issuer holds it. A Verifier compares a capability's +// one execution, as the issuer holds it. A verifier compares a capability's // sealed values against it for exact equality. type Seal = corework.Seal @@ -67,7 +82,7 @@ type SealedValues = basev0.WorkSealV1 type SealSource = corework.SealSource // RevisionSource, ReplayStore and GrantSource are the other three sources a -// Verifier requires. +// verifier requires. type ( RevisionSource = corework.RevisionSource ReplayStore = corework.ReplayStore @@ -142,7 +157,7 @@ var ( // on core's own instruction when asked: // // - ErrNeedsIssuer comes only from core's verify-only Authenticator, when - // a capability carries a grant hop. Verifier.Verify never returns it, so + // a capability carries a grant hop. Verify never returns it, so // a client matching it would be writing a branch nothing reaches. // - ErrNoSeal and ErrNoBinding are what a SealSource IMPLEMENTATION // returns, not what a client matches. diff --git a/workcontext/derived_lifetime_test.go b/workcontext/derived_lifetime_test.go new file mode 100644 index 0000000..2e32733 --- /dev/null +++ b/workcontext/derived_lifetime_test.go @@ -0,0 +1,222 @@ +package workcontext + +import ( + "fmt" + "testing" + "time" + + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + corework "github.com/codefly-dev/core/workcontext" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +// SP-WC-01 — a context derived from another, by exchange, child session or +// renewal, expires no later than its parent and never refreshes a lifetime past +// the root's. +// +// The rule holds for EVERY derivation, so it is asserted over a matrix rather +// than at one pair of numbers: an assertion at a single pair does not state a +// property. +// +// It is tested HERE, in a module that derives nothing, on purpose. Derivation is +// core's and core's alone, and a test living only in core would hold core to the +// rule; this one holds the version of core THIS module resolves to it, so the +// rule is required of whatever this module depends on rather than of core in +// general. +// +// Two halves of the invariant are not reachable from here, and saying so is part +// of the test: +// +// - At VERIFICATION, a verifier holds the capability and not its parent, and +// codefly.base.v0.WorkContextV1 carries no absolute-deadline claim and no +// per-hop timestamp — so "no later than its parent" is not a question the +// wire lets a verifier ask. That claim, and the check that reads it, are +// core's to add. What a verifier DOES enforce today is the chain's +// attenuation, in core's checkStructure. +// - A RENEWAL in this module is not a derivation at all. MintClient renews by +// presenting the projected service-account token for a fresh mint, never the +// credential it holds, so each renewal is a root the host re-authorizes +// rather than a window extended from the last one. mintLocked sends the +// audience and the projection audience and nothing else. + +// deriveChild is childAs with the TTL as a parameter, which is the dimension +// this property varies and the shared helper fixes. +func (a *authority) deriveChild( + t *testing.T, parent string, principal string, scopes []*basev0.WorkScopeV1, ttl time.Duration, +) (string, *basev0.WorkContextV1, error) { + t.Helper() + require.NoError(t, a.seals.PutEpoch(principal, 1)) + require.NoError(t, a.seals.PutApprovedBuild( + principal, testApprovedBuild.digest, testApprovedBuild.incarnation)) + verified, err := a.verifier(t).Verify(t.Context(), parent) + require.NoError(t, err) + token, claims, err := a.core.Child(t.Context(), verified, corework.ChildInput{ + Execution: corework.Execution{ + ImageDigest: testApprovedBuild.digest, + BuildIncarnation: testApprovedBuild.incarnation, + }, + PrincipalID: principal, + PrincipalKind: "service", + DelegationID: "delegation-" + principal, + GrantedScopes: scopes, + Audience: testAudience, + TTL: ttl, + }) + return token, claims, err +} + +func expiryOf(t *testing.T, a *authority, token string) time.Time { + t.Helper() + verified, err := a.verifier(t).Verify(t.Context(), token) + require.NoError(t, err) + return time.Unix(verified.Context().GetExpiresAtUnix(), 0) +} + +func TestDerivedContextNeverOutlivesParent(t *testing.T) { + parentScopes := []*basev0.WorkScopeV1{scope("document", "read", "write")} + childScopes := []*basev0.WorkScopeV1{scope("document", "read")} + + // The matrix. Every parent lifetime against every requested child window, + // including the ones that ask for more than the parent has left — which is + // the only interesting direction and the one an uncapped derivation gets + // wrong. + t.Run("a child expires no later than its parent, for every window", func(t *testing.T) { + lifetimes := []time.Duration{ + time.Minute, 5 * time.Minute, 15 * time.Minute, 30 * time.Minute, time.Hour, + } + for _, parentLifetime := range lifetimes { + for _, requested := range lifetimes { + name := fmt.Sprintf("parent %s, child asks %s", parentLifetime, requested) + t.Run(name, func(t *testing.T) { + a := newAuthority(t) + parent := a.startAt(t, a.now(), mintInput{scopes: parentScopes}, parentLifetime) + parentExpiry := expiryOf(t, a, parent) + + _, claims, err := a.deriveChild(t, parent, "principal-child", childScopes, requested) + require.NoError(t, err) + childExpiry := time.Unix(claims.GetExpiresAtUnix(), 0) + + require.False(t, childExpiry.After(parentExpiry), + "a child expiring at %s outlives a parent expiring at %s", childExpiry, parentExpiry) + // And it is the MINIMUM, not merely bounded: a derivation + // that always took the parent's expiry would satisfy the + // bound while ignoring the requested window. + wanted := a.now().Add(requested) + if wanted.After(parentExpiry) { + wanted = parentExpiry + } + require.Equal(t, wanted.Unix(), claims.GetExpiresAtUnix()) + }) + } + } + }) + + // A chain, which is where "never refreshes a lifetime past the root's" + // lives. Each hop asks for the longest window the authority mints, so the + // root's expiry is the only thing that can be holding the chain. + t.Run("a chain of derivations never outlives the root", func(t *testing.T) { + a := newAuthority(t) + root := a.startAt(t, a.now(), mintInput{scopes: parentScopes}, 2*time.Minute) + rootExpiry := expiryOf(t, a, root) + + token, previous := root, rootExpiry + for hop := 1; hop <= 4; hop++ { + derived, claims, err := a.deriveChild( + t, token, fmt.Sprintf("principal-hop-%d", hop), childScopes, time.Hour) + require.NoError(t, err) + expiry := time.Unix(claims.GetExpiresAtUnix(), 0) + + require.False(t, expiry.After(previous), "hop %d outlives the hop before it", hop) + require.False(t, expiry.After(rootExpiry), "hop %d outlives the root", hop) + require.Equal(t, rootExpiry.Unix(), claims.GetExpiresAtUnix(), + "every hop is clamped to the root's expiry once the root is the shortest window") + token, previous = derived, expiry + } + }) + + // A grant capability is the other derivation core mints, and it takes a + // different route to its expiry — bounded by the approval's own window + // first. The parent still wins when the parent is shorter. + t.Run("a grant capability expires no later than its parent", func(t *testing.T) { + a := newAuthority(t) + parent := a.startAt(t, a.now(), mintInput{scopes: parentScopes}, 2*time.Minute) + parentExpiry := expiryOf(t, a, parent) + verified, err := a.verifier(t).Verify(t.Context(), parent) + require.NoError(t, err) + + _, claims, err := a.core.Grant(t.Context(), verified, corework.GrantInput{ + Execution: corework.Execution{ + ImageDigest: testApprovedBuild.digest, + BuildIncarnation: testApprovedBuild.incarnation, + }, + // An approval whose own window closes long after the parent + // session does, asking for a window longer than either. + Grant: &corework.Grant{ + ID: "grant-1", + Approvers: []corework.Approver{{PrincipalID: testHumanPrincipal, Kind: "human"}}, + Scope: &basev0.WorkScopeV1{ResourceKind: "document", Actions: []string{"read"}, ResourceIds: []string{"document-1"}}, + Subject: "subject-1", + RequestDigest: "digest-1", + Audience: testAudience, + NotAfter: a.now().Add(12 * time.Hour), + AuthorizationRevision: 5, + }, + TTL: time.Hour, + }) + require.NoError(t, err) + + require.Equal(t, parentExpiry.Unix(), claims.GetExpiresAtUnix(), + "a grant capability outlives the session it was approved within") + }) + + // A parent already past its expiry derives NOTHING. The window here is + // inside core's skew, so the parent still verifies: that is a state a + // legitimate holder can be in, and the rule has to hold in it rather than + // only where the parent fails verification anyway. + t.Run("a parent past its expiry derives nothing", func(t *testing.T) { + a := newAuthority(t) + lifetime := 15 * time.Minute + parent := a.startAt( + t, a.now().Add(-lifetime-10*time.Second), mintInput{scopes: parentScopes}, lifetime) + require.True(t, expiryOf(t, a, parent).Before(a.now()), + "this case needs a parent that verifies and is already past its expiry") + + _, _, err := a.deriveChild(t, parent, "principal-child", childScopes, time.Hour) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "parent session expired") + }) + + // The chain never widens, and never grows by more than the hop being added. + // The parent is the bound in both dimensions: a derived context carries no + // authority its parent did not hold, and no hop its parent did not. + t.Run("a child never widens or lengthens the chain beyond one hop", func(t *testing.T) { + a := newAuthority(t) + parent := a.startAt(t, a.now(), mintInput{scopes: parentScopes}, 30*time.Minute) + + _, _, err := a.deriveChild(t, parent, "principal-wider", + []*basev0.WorkScopeV1{scope("document", "read", "write", "delete")}, time.Minute) + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "widens authority beyond its parent") + + _, _, err = a.deriveChild(t, parent, "principal-other-kind", + []*basev0.WorkScopeV1{scope("secret", "read")}, time.Minute) + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "widens authority beyond its parent") + + // One hop per derivation, and the hops before it carried through + // unchanged: a derivation that rewrote an earlier hop would be a + // chain whose earlier authority is whatever the last deriver said. + first, firstClaims, err := a.deriveChild(t, parent, "principal-first", childScopes, time.Minute) + require.NoError(t, err) + require.Len(t, firstClaims.GetActorChain(), 1) + + _, secondClaims, err := a.deriveChild(t, first, "principal-second", childScopes, time.Minute) + require.NoError(t, err) + require.Len(t, secondClaims.GetActorChain(), 2) + require.True(t, + proto.Equal(firstClaims.GetActorChain()[0], secondClaims.GetActorChain()[0]), + "deriving rewrote the hop it inherited") + }) +} diff --git a/workcontext/doc.go b/workcontext/doc.go index 4e9cc4b..8d1db65 100644 --- a/workcontext/doc.go +++ b/workcontext/doc.go @@ -9,7 +9,12 @@ // code that checks a signature, and the only encoding of either. This package // re-exports core's verification entry point rather than offering one of its // own, so a consumer that verifies through this module is verifying through -// core. +// core. What it adds around that is CONFIGURATION and never behaviour: core's +// verifier is not re-exported, NewVerifier is the only constructor and it +// refuses by name a verifier with no pinned issuer, no audience or no key set +// acquired from an admitted endpoint. ResolveKeySetEndpoint is the one transport +// guard — the host's own rule, so a plaintext hop is not a check each consumer +// writes for itself. // // That is a rule and not a preference, because the alternative was tried. A // second implementation here once signed a hand-written JSON payload while @@ -32,4 +37,8 @@ // which never spans an installation revision. // - StreamGuard: a stream re-presents its credential before every message // and terminates on refusal. +// - NewVerifier, ResolveKeySetEndpoint, AcquireKeySet and the mesh-transport +// rule beside them: the configuration a verifier is refused without — a +// pinned issuer and audience, and a key set fetched from a location admitted +// over a hop the composition accounts for. package workcontext diff --git a/workcontext/grpctransport/two_service_test.go b/workcontext/grpctransport/two_service_test.go index 3be2817..fbf3164 100644 --- a/workcontext/grpctransport/two_service_test.go +++ b/workcontext/grpctransport/two_service_test.go @@ -3,8 +3,12 @@ package grpctransport_test import ( "context" "crypto/ed25519" + "encoding/hex" "errors" "net" + "net/http" + "net/http/httptest" + "strings" "testing" "time" @@ -117,20 +121,61 @@ func newHopAuthority(t *testing.T) *hopAuthority { } } -// verifier is core's, configured for one service's audience. A verifier is -// given all four of core's sources because a verifier missing one refuses -// everything rather than skipping that check. -func (a *hopAuthority) verifier(audience string) *workcontext.Verifier { - return &workcontext.Verifier{ +// verifier is built the way a CONSUMER has to build one, which is the only way +// there is: the key-set location is admitted, the keys are fetched from it, and +// the verifier is settled on the result. A verifier is given all four of core's +// sources because one missing a source refuses everything rather than skipping +// that check. +// +// It goes through a real loopback endpoint rather than a stub, because the +// binding under test is that the keys a verifier holds are the keys an admitted +// endpoint served — a fake that handed the keys over directly would be testing +// the shape of the API and not the binding. +func (a *hopAuthority) verifier(t *testing.T, audience string) *workcontext.PinnedVerifier { + t.Helper() + endpoint := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, _ *http.Request) { + _, _ = writer.Write([]byte(hopKeyID + " " + hex.EncodeToString(a.publicKey))) + })) + t.Cleanup(endpoint.Close) + + admitted, err := workcontext.ResolveKeySetEndpoint(endpoint.URL, hopTransport{}) + require.NoError(t, err) + keys, err := workcontext.AcquireKeySet(t.Context(), admitted, hopDecodeKeySet) + require.NoError(t, err) + + verifier, err := workcontext.NewVerifier(workcontext.VerifierSettings{ Issuer: hopIssuer, Audience: audience, - Keys: map[string]ed25519.PublicKey{hopKeyID: a.publicKey}, Revisions: corework.FixedRevision(hopRevision), Replay: a.replay, Grants: hopNoGrants{}, Seals: a.seals, Now: func() time.Time { return hopNow }, + }, keys) + require.NoError(t, err) + return verifier +} + +// hopTransport is a local run with no mesh assertion, which is what an +// httptest server on loopback is. +type hopTransport struct{} + +func (hopTransport) WorkspaceValueIfSet(string, string) (string, bool, error) { return "", false, nil } +func (hopTransport) IsLocalRun() bool { return true } + +// hopDecodeKeySet is the consumer's decoder: the key set's encoding is the +// issuer's format, and reading it is the consumer's job because it is where +// naming a signature primitive is legitimate. +func hopDecodeKeySet(payload []byte) (map[string][]byte, error) { + id, encoded, found := strings.Cut(string(payload), " ") + if !found { + return nil, errors.New("not a key set") + } + key, err := hex.DecodeString(encoded) + if err != nil { + return nil, err } + return map[string][]byte{id: key}, nil } type hopNoGrants struct{} @@ -143,7 +188,7 @@ func (hopNoGrants) Grant(context.Context, string) (*corework.Grant, error) { // carrier from gRPC metadata, verified for this service's own audience. func verifyIncoming( ctx context.Context, - verifier *workcontext.Verifier, + verifier *workcontext.PinnedVerifier, ) (grpctransport.ExecutionContext, *workcontext.Verified, error) { execution, err := grpctransport.GRPCExecutionContextFromIncoming(ctx) if err != nil { @@ -157,7 +202,7 @@ func verifyIncoming( } type serviceB struct { - verifier *workcontext.Verifier + verifier *workcontext.PinnedVerifier accepted chan *workcontext.Verified } @@ -171,7 +216,7 @@ func (s *serviceB) call(ctx context.Context, _ *healthv1.HealthCheckRequest) (*h } type serviceA struct { - verifier *workcontext.Verifier + verifier *workcontext.PinnedVerifier authority *hopAuthority b grpc.ClientConnInterface accepted chan *workcontext.Verified @@ -282,9 +327,9 @@ func newHopTopology(t *testing.T) *hopTopology { atA: make(chan *workcontext.Verified, 1), atB: make(chan *workcontext.Verified, 1), } - topology.b = serveHop(t, &serviceB{verifier: authority.verifier(audienceB), accepted: topology.atB}) + topology.b = serveHop(t, &serviceB{verifier: authority.verifier(t, audienceB), accepted: topology.atB}) topology.a = serveHop(t, &serviceA{ - verifier: authority.verifier(audienceA), authority: authority, b: topology.b, accepted: topology.atA, + verifier: authority.verifier(t, audienceA), authority: authority, b: topology.b, accepted: topology.atA, }) return topology } diff --git a/workcontext/minting_test.go b/workcontext/minting_test.go index b7933a9..1b8642b 100644 --- a/workcontext/minting_test.go +++ b/workcontext/minting_test.go @@ -126,10 +126,10 @@ func newAuthorityOver(t *testing.T, seals *corework.MemorySealSource) *authority // verifier is core's verifier configured for this authority — the module's only // verification entry point, reached through the alias in core.go. -func (a *authority) verifier(t *testing.T) *Verifier { +func (a *authority) verifier(t *testing.T) *verifier { t.Helper() public, _ := corework.FixtureKeyPair() - return &Verifier{ + return &verifier{ Issuer: testIssuer, Audience: testAudience, Keys: map[string]ed25519.PublicKey{testKeyID: public}, diff --git a/workcontext/one_implementation_test.go b/workcontext/one_implementation_test.go index 512fe7f..7c0d4b4 100644 --- a/workcontext/one_implementation_test.go +++ b/workcontext/one_implementation_test.go @@ -26,7 +26,7 @@ import ( // core's pointer type, and the gate below would be guarding a surface that had // already forked. var ( - _ *corework.Verifier = (*Verifier)(nil) + _ *corework.Verifier = (*verifier)(nil) _ *corework.Verified = (*Verified)(nil) _ corework.Seal = Seal{} _ corework.OperationBinding = OperationBinding{} @@ -1896,7 +1896,7 @@ func parseSource(t *testing.T, path string, source string) sourceFile { // the misdiagnosis this whole rule exists to prevent. func TestWorkContextConformance(t *testing.T) { settings := conformance.New(time.Now()) - exported := &Verifier{ + exported := &verifier{ Issuer: settings.Issuer, Audience: settings.Audience, // Settings.Keys is map[string][]byte so holding settings needs no diff --git a/workcontext/parse_agreement_test.go b/workcontext/parse_agreement_test.go index d6e0f52..9463c31 100644 --- a/workcontext/parse_agreement_test.go +++ b/workcontext/parse_agreement_test.go @@ -70,7 +70,7 @@ var structurallyRefused = map[string]bool{ // rather than quietly dropping out of coverage. // // This is the test the conformance test is not. Driving fixtures through -// `&Verifier{}` exercises core, because Verifier is an alias of core's type — +// core.s verifier directly exercises core, because this package.s is an alias — // it is core testing core, and it would have passed unchanged beside the // implementation this PR deletes. The SDK's own decision paths are // readClaims/sealOf, SealedInstallation, FromHeaders and credentialFrom, and that diff --git a/workcontext/stream.go b/workcontext/stream.go index 5feacfc..5e4e137 100644 --- a/workcontext/stream.go +++ b/workcontext/stream.go @@ -18,7 +18,7 @@ type StreamGuardOptions struct { // Recheck re-reads the issuer's live state for a capability that is ALREADY // verified, and returns the host's answer. // - // Use core's (*Verifier).Recheck. Do NOT use Verify: it consumes a + // Use (*PinnedVerifier).Recheck. Do NOT use Verify: it consumes a // single-use nonce, and every grant capability is single-use, so a stream // opened with one died with ErrReplayed at its first message. This // module's own README recommended exactly that for a while. Re-checking @@ -41,7 +41,7 @@ type StreamGuardOptions struct { // everybody knows, and a guard built on it works in every test that does not // use a single-use capability and then kills the first grant-opened stream in // production. -func RecheckWith(verifier *Verifier, verified *Verified) func(context.Context) error { +func RecheckWith(verifier *PinnedVerifier, verified *Verified) func(context.Context) error { return func(ctx context.Context) error { if verifier == nil || verified == nil { return fmt.Errorf("%w: a stream re-check needs a verifier and a verified capability", ErrInvalid) diff --git a/workcontext/stream_test.go b/workcontext/stream_test.go index d9927e4..c69f42e 100644 --- a/workcontext/stream_test.go +++ b/workcontext/stream_test.go @@ -170,7 +170,7 @@ func TestStreamGuardValidatesItsConfiguration(t *testing.T) { // call instead of the one that compiles just as well and breaks later. func TestTheStreamRecheckDoesNotConsumeASingleUseCapability(t *testing.T) { settings := conformance.New(time.Now()) - verifier := &Verifier{ + verifier := &verifier{ Issuer: settings.Issuer, Audience: settings.Audience, Keys: settings.PublicKeys(), @@ -198,7 +198,7 @@ func TestTheStreamRecheckDoesNotConsumeASingleUseCapability(t *testing.T) { // Now the stream, built the way this module recommends. guard, err := NewStreamGuard(StreamGuardOptions{ - Recheck: RecheckWith(verifier, verified), + Recheck: RecheckWith(pinnedFor(verifier), verified), }) require.NoError(t, err) for message := range 5 { diff --git a/workcontext/verifier.go b/workcontext/verifier.go new file mode 100644 index 0000000..80e7a0e --- /dev/null +++ b/workcontext/verifier.go @@ -0,0 +1,635 @@ +package workcontext + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "slices" + "strings" + "time" +) + +// MeshTransportGroup and MeshTransportKey name the workspace setting a +// composition asserts the cell's mesh with. +// +// In-cell transport security is the mesh's: every hop between workloads on a +// cell is authenticated and encrypted by the platform mesh, so a service +// terminates no TLS of its own for an in-cell peer. A composition states that +// out loud, and a service accepts plaintext to an in-cluster Service address +// exactly under that assertion and refuses it otherwise, by name. +// +// This is the same group, key and remedy sentence the host and the composed +// modules apply to their own in-cluster hops. It is reimplemented here rather +// than imported — a module never imports another module's internals — so the +// shared thing is the contract and each side owns its copy of the check. This +// copy is the SDK's, which is what lets a consumer route every plaintext hop it +// has through one check instead of writing a fourth. +const ( + MeshTransportGroup = "internal-transport" + MeshTransportKey = "mesh-protected" +) + +// MeshTransportAssertion is the one value that asserts the mesh. It is compared +// LITERALLY: see MeshProtected. +const MeshTransportAssertion = "true" + +// MeshTransportRemedy is the sentence every plaintext refusal ends with, so an +// operator reads the one supported way through off the error rather than off a +// document. +const MeshTransportRemedy = "use https, or — only for an in-cluster Service address " + + "(..svc, optionally followed by cluster.local) when every in-cluster hop " + + "is carried by a mutually authenticated mesh (mTLS) — assert it with the workspace setting " + + MeshTransportGroup + "/" + MeshTransportKey + ", whose value must be exactly " + + MeshTransportAssertion + " with nothing around it" + +// TransportSource answers the two things an admission decision needs, and +// answers both from the runtime rather than from an argument a caller chooses: +// the composition's mesh assertion, and whether this is a local run. +// +// `codefly.For(ctx)` satisfies it. That is the point of its being an interface +// of two accessors rather than two parameters: a boolean parameter is a +// per-service opt-in that no address has to qualify for, which is the shape the +// host deleted. Here the only way to say plaintext is permitted is for the +// composition to have said it. +// +// WorkspaceValueIfSet rather than WorkspaceValue, because the three answers +// have to stay three. A setting that is NOT CONFIGURED is an answer — the mesh +// is not asserted. A setting whose delivery could not be read, or whose +// authority-bearing value has drifted since it was pinned at boot, is not an +// answer at all, and reading it as "not asserted" would be a lookup failure +// quietly deciding a posture question. +type TransportSource interface { + // WorkspaceValueIfSet resolves a workspace setting: the value and true when + // it is set, ("", false, nil) when nothing carries it, and an error when the + // lookup failed. + WorkspaceValueIfSet(name string, key string) (string, bool, error) + + // IsLocalRun reports whether the Codefly environment is local. + IsLocalRun() bool +} + +// MeshProtected reports whether the composition asserted that the cell's mesh +// protects plaintext in-cluster hops. +// +// Fail-closed on three counts. +// +// ONLY THE LITERAL VALUE "true" ASSERTS IT. Not " true", not "true\n", not a +// value carrying any other whitespace: the comparison is byte for byte. An +// earlier revision trimmed first, on the reasoning that a value delivered by +// file carries the newline the file ends with — but TrimSpace trims every +// Unicode space, so values that are not the assertion asserted it, and the +// delivery's own shape is the delivery's problem to fix rather than this +// check's to absorb. +// +// Unset and "false" keep plaintext refused. ANY OTHER VALUE IS AN ERROR rather +// than being read as either answer, because a misspelled assertion must not pass +// for an absent one nor for a present one. +// +// A LOOKUP FAILURE IS NOT ABSENCE. An unreadable delivery, or a value that has +// drifted from the one pinned at boot, is returned as an error; only a setting +// nothing carries is "not asserted". +// +// No refusal here echoes the value. WorkspaceValueIfSet reads the public +// namespace and then the secret one, so the value may be a credential, and a +// configuration error reaches boot-time logging. +func MeshProtected(source TransportSource) (bool, error) { + if source == nil { + return false, fmt.Errorf("%w: admission needs a transport source", ErrInvalid) + } + value, set, err := source.WorkspaceValueIfSet(MeshTransportGroup, MeshTransportKey) + if err != nil { + return false, fmt.Errorf( + "%w: %s/%s could not be read, which is not the same as not being asserted: %w", + ErrInvalid, MeshTransportGroup, MeshTransportKey, redactedLookupFailure(err)) + } + if !set { + return false, nil + } + switch value { + case MeshTransportAssertion: + return true, nil + case "", "false": + // An empty value is "not asserted", like an absent one. It is the one + // value that is not a misspelling: nothing was said. + return false, nil + default: + return false, fmt.Errorf( + "%w: %s/%s is set to something other than %q or %q, and is compared byte for byte; "+ + "its value is not reported because this setting is read from the secret namespace too", + ErrInvalid, MeshTransportGroup, MeshTransportKey, MeshTransportAssertion, "false") + } +} + +// redactedLookupFailure keeps a source's error in the chain for errors.Is while +// keeping its text out of the message, because a lookup error may name the value +// it could not use. +func redactedLookupFailure(err error) error { + return lookupFailure{err: err} +} + +type lookupFailure struct{ err error } + +func (f lookupFailure) Error() string { return "the workspace lookup failed" } +func (f lookupFailure) Unwrap() error { return f.err } + +// ClusterServiceHost reports whether hostname names a Kubernetes Service inside +// this cluster. It accepts exactly two shapes: +// +// ..svc +// ..svc.cluster.local +// +// and nothing else. Both labels before `svc` must be valid DNS-1123 labels. +// +// The suffix is matched against `cluster.local` literally rather than accepted +// as "whatever follows svc", because an arbitrary suffix is not evidence of a +// cluster domain: a name like `..svc.example.com` is an +// ordinary public name that happens to carry an `svc` label, and admitting it +// would let the mesh assertion cover a destination the mesh does not. Nothing +// in the platform supplies an authoritative cluster-domain value to compare +// against, so the trusted suffix is Kubernetes' default and only that; a cell +// with a custom cluster domain uses the unqualified `..svc` +// form, which resolves in-cluster under any domain. If a configuration seam for +// the cluster domain ever exists, this is where it is read — until then, +// inferring one is the bug. +// +// It takes a HOSTNAME and not a host: a port is the caller's to strip, which +// url.URL.Hostname does, so there is no second parse of an authority here. +func ClusterServiceHost(hostname string) bool { + labels := strings.Split(strings.TrimSuffix(strings.ToLower(hostname), "."), ".") + switch len(labels) { + case 3: + case 5: + if labels[3] != "cluster" || labels[4] != "local" { + return false + } + default: + return false + } + if labels[2] != "svc" { + return false + } + return dnsLabel(labels[0]) && dnsLabel(labels[1]) +} + +// dnsLabel reports whether label is a valid DNS-1123 label, so an empty or +// malformed component cannot ride through on the shape of the name alone. +func dnsLabel(label string) bool { + if len(label) == 0 || len(label) > 63 { + return false + } + for index := range len(label) { + character := label[index] + switch { + case character >= 'a' && character <= 'z', character >= '0' && character <= '9': + case character == '-' && index != 0 && index != len(label)-1: + default: + return false + } + } + return true +} + +// keySetLoopbackHosts are the hosts a local run may name over plaintext. +// +// An EXACT set, by spelling, because what a name resolves to is a +// resolution-time question this cannot ask. `localhost` is reserved to the +// loopback interface by RFC 6761, so that is a property of the name; a name +// that merely contains one of these spellings is a different host. +var keySetLoopbackHosts = []string{"127.0.0.1", "::1", "localhost"} + +// KeySetEndpoint is a key-set location whose transport has been admitted. +// +// It is an opaque value with an unexported field and exactly one constructor, +// so the location a key set is fetched from is the location that was admitted. +type KeySetEndpoint struct { + url string +} + +// URL is the location the key set is fetched from. It is empty on a zero value, +// which is not an endpoint. +func (e KeySetEndpoint) URL() string { return e.url } + +// String implements fmt.Stringer. +func (e KeySetEndpoint) String() string { return e.url } + +// ResolveKeySetEndpoint admits a key-set location, or refuses it by name. +// +// A verifier's key set decides which signatures it accepts, so the hop it +// arrives over is part of the verifier's configuration rather than a detail of +// how it was fetched. This is the one place that decision is made; a consumer +// that writes its own is a second copy of a rule whose whole value is that there +// is one. +// +// What is admitted: +// +// - https to any host. +// - plaintext http to an in-cluster Service address, exactly under the +// composition's mesh assertion. Neither half admits on its own: the +// assertion covers only what a mesh can cover, so a plaintext URL to any +// other host stays refused with it set, and an in-cluster address without it +// is refused too. +// - plaintext http to a loopback host, on a local run only. A deployed runtime +// has one admission rule and loopback is not an exception to it, not even +// with the assertion set: a mesh does not carry a hop that never leaves the +// pod, so nothing the composition said covers it. +// +// The assertion is read and validated FIRST, whatever the URL's scheme, so a +// misspelled or unreadable assertion stops the process rather than sitting +// unread behind an https URL until the day somebody changes one. +// +// No refusal here echoes any part of the URL, the parser's message included. A +// configuration error reaches boot-time logging, and a URL is a place a +// credential or a token gets written by mistake; every refusal names the setting +// and the remedy instead. +func ResolveKeySetEndpoint(raw string, source TransportSource) (KeySetEndpoint, error) { + protected, err := MeshProtected(source) + if err != nil { + return KeySetEndpoint{}, err + } + trimmed := strings.TrimSpace(raw) + if trimmed == "" { + return KeySetEndpoint{}, fmt.Errorf( + "%w: no key-set URL is configured, so the key set is unset", ErrInvalid) + } + parsed, err := url.Parse(trimmed) + if err != nil || parsed.Opaque != "" || parsed.Hostname() == "" { + // The parser's error text embeds the input, so it is not reported. + return KeySetEndpoint{}, fmt.Errorf( + "%w: the key-set URL is not an absolute URL naming a host", ErrInvalid) + } + if parsed.User != nil || parsed.RawQuery != "" || parsed.ForceQuery || parsed.Fragment != "" { + return KeySetEndpoint{}, fmt.Errorf( + "%w: the key-set URL must carry no credentials, query or fragment", ErrInvalid) + } + if parsed.Scheme == "https" { + return KeySetEndpoint{url: parsed.String()}, nil + } + if parsed.Scheme != "http" { + return KeySetEndpoint{}, fmt.Errorf( + "%w: the key-set URL scheme is not https; %s", ErrInvalid, MeshTransportRemedy) + } + if slices.Contains(keySetLoopbackHosts, strings.ToLower(parsed.Hostname())) { + if source.IsLocalRun() { + return KeySetEndpoint{url: parsed.String()}, nil + } + return KeySetEndpoint{}, fmt.Errorf( + "%w: the key-set URL is plaintext to a loopback host and this is not a local run; "+ + "a mesh carries no hop that never leaves the pod, so the assertion cannot cover it. %s", + ErrInvalid, MeshTransportRemedy) + } + if !protected { + return KeySetEndpoint{}, fmt.Errorf( + "%w: the key-set URL is plaintext and %s/%s is not asserted. %s", + ErrInvalid, MeshTransportGroup, MeshTransportKey, MeshTransportRemedy) + } + if !ClusterServiceHost(parsed.Hostname()) { + return KeySetEndpoint{}, fmt.Errorf( + "%w: %s/%s is asserted and the key-set URL is plaintext to a host that is not an "+ + "in-cluster Service address, which the assertion does not cover. %s", + ErrInvalid, MeshTransportGroup, MeshTransportKey, MeshTransportRemedy) + } + return KeySetEndpoint{url: parsed.String()}, nil +} + +// ErrKeySetUnavailable is a key set that could not be fetched: the endpoint did +// not answer, answered with a status, or answered with more than this module +// reads. It is retryable, and it is a different condition from ErrInvalid, +// which is a configuration this process will produce again. +var ErrKeySetUnavailable = errors.New("Codefly Work Context key set unavailable") + +const ( + // maxKeySetBytes bounds what is read from the endpoint. A key set is a few + // kilobytes; anything larger is not one, and reading it unbounded makes the + // endpoint able to exhaust this process. + maxKeySetBytes = 256 * 1024 + + // maxKeySetKeys bounds how many keys a verifier will hold. + maxKeySetKeys = 64 + + // keySetRequestTimeout bounds one fetch. The caller's context bounds it too; + // this is the ceiling when the caller supplies none. + keySetRequestTimeout = 10 * time.Second +) + +// KeySetDecoder turns the bytes the endpoint served into verification keys by +// key id. +// +// The decoding is the CALLER'S and the keys come back as plain bytes, +// deliberately: a key set's encoding is the issuer's format, and reading one +// means naming a signature primitive, which is the one thing this module may +// never do — see workcontext/AGENTS.md. What this module owns is the part a +// decoder cannot: that the bytes came from an admitted endpoint, and that +// nothing but those bytes reaches a verifier. +type KeySetDecoder func(payload []byte) (map[string][]byte, error) + +// KeySet is verification key material that was fetched from an admitted +// endpoint. It is opaque with exactly one constructor, so the keys a verifier +// holds are the keys that endpoint served — the endpoint check and the fetch are +// one operation rather than two a caller could do separately and then ignore. +type KeySet struct { + endpoint string + keys map[string][]byte +} + +// Endpoint is the admitted location the keys were fetched from. It is empty on a +// zero value, which is not a key set. +func (s KeySet) Endpoint() string { return s.endpoint } + +// KeyIDs are the ids the endpoint served, sorted. +func (s KeySet) KeyIDs() []string { + ids := make([]string, 0, len(s.keys)) + for id := range s.keys { + ids = append(ids, id) + } + slices.Sort(ids) + return ids +} + +// AcquireKeySet fetches the key set from an admitted endpoint and decodes it +// through the caller's decoder. +// +// It is the only way to obtain a KeySet, and NewVerifier takes nothing else, so +// a verifier's keys cannot be supplied beside an endpoint that was checked and +// then not used. The endpoint check answering "this location is permitted" while +// the keys came from somewhere else is two facts presented as one. +// +// The HTTP client is this module's and a caller supplies none, on the same terms +// as the mint client's: a redirect is refused rather than followed, because +// following one leaves the address that was admitted; the body is bounded; and +// nothing is sent but the request. +func AcquireKeySet(ctx context.Context, endpoint KeySetEndpoint, decode KeySetDecoder) (KeySet, error) { + if ctx == nil { + return KeySet{}, fmt.Errorf("%w: acquiring a key set needs a context", ErrInvalid) + } + if endpoint.URL() == "" { + return KeySet{}, fmt.Errorf( + "%w: acquiring a key set needs an admitted endpoint; resolve one with ResolveKeySetEndpoint", + ErrInvalid) + } + if decode == nil { + return KeySet{}, fmt.Errorf("%w: acquiring a key set needs a decoder", ErrInvalid) + } + requestContext, cancel := context.WithTimeout(ctx, keySetRequestTimeout) + defer cancel() + request, err := http.NewRequestWithContext(requestContext, http.MethodGet, endpoint.URL(), nil) + if err != nil { + return KeySet{}, fmt.Errorf("%w: the key-set endpoint could not be requested", ErrInvalid) + } + client := &http.Client{ + Timeout: keySetRequestTimeout, + CheckRedirect: refuseKeySetRedirect, + } + response, err := client.Do(request) + if err != nil { + // A refused redirect arrives here already classed, wrapped in a + // *url.Error whose text is the address; it is named here rather than + // reported through. + if errors.Is(err, errKeySetRedirected) { + return KeySet{}, fmt.Errorf( + "%w: the key-set endpoint redirected, which leaves the admitted address", + ErrKeySetUnavailable) + } + // Every other transport error names the URL it was given too, so it is + // kept in the chain and out of the message. + return KeySet{}, fmt.Errorf("%w: the key-set endpoint did not answer: %w", + ErrKeySetUnavailable, redactedLookupFailure(err)) + } + defer func() { _ = response.Body.Close() }() + if response.StatusCode != http.StatusOK { + return KeySet{}, fmt.Errorf("%w: the key-set endpoint answered %d", + ErrKeySetUnavailable, response.StatusCode) + } + payload, err := io.ReadAll(io.LimitReader(response.Body, maxKeySetBytes+1)) + if err != nil { + return KeySet{}, fmt.Errorf("%w: the key set could not be read", ErrKeySetUnavailable) + } + if len(payload) > maxKeySetBytes { + return KeySet{}, fmt.Errorf("%w: the key set is larger than %d bytes", + ErrKeySetUnavailable, maxKeySetBytes) + } + decoded, err := decode(payload) + if err != nil { + // A decoder is the caller's code and its error is the caller's text, so + // it stays in the chain rather than in the message. + return KeySet{}, fmt.Errorf("%w: the key set could not be decoded: %w", + ErrInvalid, redactedLookupFailure(err)) + } + if len(decoded) == 0 { + return KeySet{}, fmt.Errorf("%w: the key-set endpoint served no key", ErrInvalid) + } + if len(decoded) > maxKeySetKeys { + return KeySet{}, fmt.Errorf("%w: the key set holds %d keys and a verifier holds at most %d", + ErrInvalid, len(decoded), maxKeySetKeys) + } + keys := make(map[string][]byte, len(decoded)) + for id, key := range decoded { + if id == "" { + return KeySet{}, fmt.Errorf("%w: the key set holds a key with no id", ErrInvalid) + } + keys[id] = key + } + return KeySet{endpoint: endpoint.URL(), keys: keys}, nil +} + +// errKeySetRedirected is how a refused redirect travels back out of the HTTP +// client, which wraps it in a *url.Error whose text is the URL. It is matched +// rather than reported, so the refusal names the condition and not the address. +var errKeySetRedirected = fmt.Errorf("%w: redirected", ErrKeySetUnavailable) + +// refuseKeySetRedirect stops a fetch that would leave the admitted address. A +// redirect is the endpoint choosing where the key set comes from, which is the +// decision ResolveKeySetEndpoint exists to make. +func refuseKeySetRedirect(*http.Request, []*http.Request) error { + return errKeySetRedirected +} + +// verificationKeySize is the length core requires of a verification key. +// +// It is a NUMBER here and a call in core — resolveKey compares against +// ed25519.PublicKeySize — which is a duplication this module would rather not +// have. It has it because naming a signature primitive is the one thing this +// module may never do, and core exports no key-configuration validator to call +// instead. So the number is stated once and TestTheVerificationKeySizeAgreesWithCore +// holds it to core's own constant from a _test.go, where the import is +// legitimate. If core grows the validator this becomes a call and the number +// goes. The same arrangement, for the same reason, as sealCarriesAnExecution. +const verificationKeySize = 32 + +// VerifierSettings are everything a verifier is, besides its key set. +// +// Every field of core's verifier has a usable zero value, so an incomplete +// configuration is expressible and the invariant requires it to be refused, and +// refused BY NAME. Failing closed and failing closed by name are two different +// properties and this is the second. +// +// Nothing here is optional except the clock and the skew, which are core's own +// test seams, and nothing here turns a check off. +type VerifierSettings struct { + // Issuer is the authority this verifier trusts. A capability from any other + // issuer is not a credential here, whatever its signature checks out + // against. + // + // It is compared byte for byte by core, so it is taken byte for byte here: + // a value with surrounding whitespace is refused rather than trimmed, since + // the two spellings are two pins. + Issuer string + + // Audience is this service or trust boundary, on the same terms. + Audience string + + // Revisions answers the issuer's current authorization revision. + Revisions RevisionSource + + // Replay consumes single-use capabilities. + Replay ReplayStore + + // Grants resolves the approval a grant capability claims. + Grants GrantSource + + // Seals answers the live installation, epoch, build and binding state. + Seals SealSource + + // Now is the clock. nil means time.Now. + Now func() time.Time + + // Skew is the tolerance applied to a capability's window. Zero means core's + // DefaultSkew. + Skew time.Duration +} + +// PinnedVerifier is a verifier whose issuer, audience and key set are settled. +// It is the only thing this package hands out with a Verify method. +// +// It holds core's verifier and adds nothing to verification — Verify and Recheck +// are one line each and must stay that way. The type exists so that the +// configuration cannot be skipped, not so that behaviour can be added: there is +// exactly one implementation of the capability and it is core's. +type PinnedVerifier struct { + verifier *verifier + keySet KeySet +} + +// NewVerifier settles a verifier's configuration or refuses it by name. +// +// What it refuses, each one named in the error: +// +// - no issuer, no audience, or either carrying surrounding whitespace; +// - a key set that was not acquired from an admitted endpoint (a zero KeySet); +// - a key set holding no key, or key material of the wrong length, which core +// refuses per request and which belongs in a refusal at boot; +// - a missing revision source, replay store, grant source or seal source, each +// by its own name, where core names all four in one message. +// +// There is no way to hand it a key set from anywhere but AcquireKeySet, and no +// way to reach core's verifier through this package, so "the endpoint was +// admitted" and "these are the keys in use" are one fact rather than two. +func NewVerifier(settings VerifierSettings, keys KeySet) (*PinnedVerifier, error) { + issuer, err := pinnedValue("issuer", settings.Issuer) + if err != nil { + return nil, err + } + audience, err := pinnedValue("audience", settings.Audience) + if err != nil { + return nil, err + } + if keys.Endpoint() == "" { + return nil, fmt.Errorf( + "%w: this verifier has no key set from an admitted endpoint; acquire one with "+ + "ResolveKeySetEndpoint and AcquireKeySet", ErrInvalid) + } + if len(keys.keys) == 0 { + return nil, fmt.Errorf("%w: the key set holds no verification key", ErrInvalid) + } + for _, id := range keys.KeyIDs() { + if size := len(keys.keys[id]); size != verificationKeySize { + return nil, fmt.Errorf( + "%w: verification key %q is %d bytes and a verification key is %d; "+ + "a key set carrying unusable material is refused here rather than per request", + ErrInvalid, id, size, verificationKeySize) + } + } + for _, source := range []struct { + what string + missing bool + }{ + {"revision source", settings.Revisions == nil}, + {"replay store", settings.Replay == nil}, + {"grant source", settings.Grants == nil}, + {"seal source", settings.Seals == nil}, + } { + if source.missing { + return nil, fmt.Errorf("%w: this verifier has no %s", ErrInvalid, source.what) + } + } + pinned := &PinnedVerifier{ + verifier: &verifier{ + Issuer: issuer, + Audience: audience, + Revisions: settings.Revisions, + Replay: settings.Replay, + Grants: settings.Grants, + Seals: settings.Seals, + Now: settings.Now, + Skew: settings.Skew, + }, + keySet: keys, + } + fillVerificationKeys(&pinned.verifier.Keys, keys.keys) + return pinned, nil +} + +// fillVerificationKeys moves the acquired bytes into the verifier's key map. +// +// It is generic so the element type is INFERRED from the field it fills: this +// module may not name the signature primitive that type is, and a conversion +// through a type parameter whose core type is []byte names nothing. +func fillVerificationKeys[K ~[]byte](into *map[string]K, acquired map[string][]byte) { + keys := make(map[string]K, len(acquired)) + for id, key := range acquired { + keys[id] = K(key) + } + *into = keys +} + +// Verify checks a presented capability. It is core's Verify and nothing else; +// this method adds no check and must never. +func (p *PinnedVerifier) Verify(ctx context.Context, encoded string) (*Verified, error) { + if p == nil { + return nil, fmt.Errorf("%w: nil verifier", ErrInvalid) + } + return p.verifier.Verify(ctx, encoded) +} + +// Recheck re-reads the issuer's live state for a capability that is already +// verified. It is core's Recheck and nothing else. +func (p *PinnedVerifier) Recheck(ctx context.Context, verified *Verified) error { + if p == nil { + return fmt.Errorf("%w: nil verifier", ErrInvalid) + } + return p.verifier.Recheck(ctx, verified) +} + +// Issuer, Audience and KeySetEndpoint report what this verifier was settled +// with, for a diagnostic or a readiness check. +func (p *PinnedVerifier) Issuer() string { return p.verifier.Issuer } +func (p *PinnedVerifier) Audience() string { return p.verifier.Audience } + +// KeySetEndpoint is the admitted location this verifier's keys came from. +func (p *PinnedVerifier) KeySetEndpoint() string { return p.keySet.Endpoint() } + +// pinnedValue refuses an unset pin, and a pin whose spelling is not the +// spelling it will be compared under. +func pinnedValue(what string, value string) (string, error) { + if value == "" { + return "", fmt.Errorf("%w: no %s is pinned", ErrInvalid, what) + } + if strings.TrimSpace(value) != value { + return "", fmt.Errorf( + "%w: the pinned %s carries surrounding whitespace; it is compared byte for byte", + ErrInvalid, what) + } + return value, nil +} diff --git a/workcontext/verifier_test.go b/workcontext/verifier_test.go new file mode 100644 index 0000000..7fafeef --- /dev/null +++ b/workcontext/verifier_test.go @@ -0,0 +1,842 @@ +package workcontext + +import ( + "crypto/ed25519" + "encoding/hex" + "errors" + "fmt" + "go/ast" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + "time" + + corework "github.com/codefly-dev/core/workcontext" + "github.com/stretchr/testify/require" +) + +// SP-WC-05 — a verifier accepts a capability only when it carries the configured +// issuer and this module's own audience; it reads its key set only over +// mesh-protected transport, and fails closed BY NAME when the issuer or the key +// set is unset. +// +// Verification is core's. What is required here is that no verifier exists +// without that configuration, that each missing part is refused by its own name, +// and that the keys a verifier holds are the keys an admitted endpoint served. + +// --------------------------------------------------------------------------- +// The transport source: the composition's assertion and the runtime. +// --------------------------------------------------------------------------- + +type transport struct { + assertion string + set bool + failure error + local bool +} + +func (t transport) WorkspaceValueIfSet(name string, key string) (string, bool, error) { + if name != MeshTransportGroup || key != MeshTransportKey { + return "", false, nil + } + if t.failure != nil { + return "", false, t.failure + } + return t.assertion, t.set, nil +} + +func (t transport) IsLocalRun() bool { return t.local } + +var ( + meshed = transport{assertion: "true", set: true} + unasserted = transport{} + localRun = transport{local: true} + meshedLocal = transport{assertion: "true", set: true, local: true} +) + +// --------------------------------------------------------------------------- +// A real endpoint, because the binding under test is that the keys came from one. +// --------------------------------------------------------------------------- + +// decodeKeySet is a consumer's decoder. Reading a key set means naming a +// signature primitive, which is why the decoding is the caller's and the keys +// cross this module's boundary as plain bytes. +func decodeKeySet(payload []byte) (map[string][]byte, error) { + keys := map[string][]byte{} + for _, line := range strings.Split(strings.TrimSpace(string(payload)), "\n") { + id, encoded, found := strings.Cut(line, " ") + if !found { + return nil, errors.New("not a key set") + } + key, err := hex.DecodeString(encoded) + if err != nil { + return nil, err + } + keys[id] = key + } + return keys, nil +} + +// servedKeySet stands up a loopback endpoint serving body, and returns the +// admitted endpoint for it. +func servedKeySet(t *testing.T, body string) KeySetEndpoint { + t.Helper() + return servedBy(t, func(writer http.ResponseWriter, _ *http.Request) { + _, _ = writer.Write([]byte(body)) + }) +} + +func servedBy(t *testing.T, handler http.HandlerFunc) KeySetEndpoint { + t.Helper() + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + endpoint, err := ResolveKeySetEndpoint(server.URL+"/keys", localRun) + require.NoError(t, err) + return endpoint +} + +func keySetLine(id string, key []byte) string { return id + " " + hex.EncodeToString(key) } + +// acquired is a usable key set holding one key of the right length. +func acquired(t *testing.T, key ed25519.PublicKey) KeySet { + t.Helper() + set, err := AcquireKeySet(t.Context(), servedKeySet(t, keySetLine(testKeyID, key)), decodeKeySet) + require.NoError(t, err) + return set +} + +func mustKey(t *testing.T) ed25519.PublicKey { + t.Helper() + public, _, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + return public +} + +// signingAuthority is core's minter with a REAL signing key: a verifier built +// the only way this package allows holds whatever key the endpoint served, so a +// test needs a key whose private half it has. +type signingAuthority struct { + *authority + public ed25519.PublicKey +} + +func newSigningAuthority(t *testing.T) signingAuthority { + t.Helper() + over := newAuthority(t) + public, private, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + minter := *over.core + minter.Key = private + return signingAuthority{ + authority: &authority{core: &minter, seals: over.seals, now: over.now}, + public: public, + } +} + +func settingsFor(a *authority) VerifierSettings { + return VerifierSettings{ + Issuer: testIssuer, + Audience: testAudience, + Revisions: corework.FixedRevision(5), + Replay: corework.NewMemoryReplayStore(), + Grants: noGrants{}, + Seals: a.seals, + Now: a.now, + } +} + +// pinnedFor wraps a raw verifier for the in-package tests that build one from +// core's conformance kit. It exists only here: there is no exported way to make +// one, which is the property TestEveryVerificationPathIsPinned holds. +func pinnedFor(v *verifier) *PinnedVerifier { return &PinnedVerifier{verifier: v} } + +// --------------------------------------------------------------------------- +// The number this module states and core calls for. +// --------------------------------------------------------------------------- + +func TestTheVerificationKeySizeAgreesWithCore(t *testing.T) { + require.Equal(t, ed25519.PublicKeySize, verificationKeySize, + "verificationKeySize is this module's copy of the length core's resolveKey requires; "+ + "if core changed it, this number changes with it or every usable key set is refused") +} + +// --------------------------------------------------------------------------- +// The mesh assertion: literal, three-valued, and never echoed. +// --------------------------------------------------------------------------- + +// ONLY THE LITERAL VALUE asserts the mesh. A value carrying any whitespace is +// not that value, and is an error rather than either answer. +func TestMeshProtectedComparesTheAssertionLiterally(t *testing.T) { + protected, err := MeshProtected(transport{assertion: "true", set: true}) + require.NoError(t, err) + require.True(t, protected) + + for _, value := range []string{ + " true", "true ", "\ttrue", "true\n", "true\r\n", " true", "true ", + "TRUE", "True", "1", "yes", "0", "no", "FALSE", "tru", "true false", + } { + t.Run(fmt.Sprintf("refuses %q", value), func(t *testing.T) { + _, err := MeshProtected(transport{assertion: value, set: true}) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, MeshTransportGroup+"/"+MeshTransportKey) + }) + } +} + +// The refusal names the setting and the permitted values and nothing else. +// WorkspaceValueIfSet reads the secret namespace too, so the value may be a +// credential, and a configuration error reaches boot-time logging. +func TestTheAssertionRefusalDoesNotEchoTheValue(t *testing.T) { + const marker = "zzsecretzz" + for _, value := range []string{marker, "true " + marker, "AKIA" + marker, "hunter2-" + marker} { + _, err := MeshProtected(transport{assertion: value, set: true}) + + require.Error(t, err) + require.NotContains(t, err.Error(), marker, + "the refusal for %q echoes the configured value", value) + } +} + +// Unset, and the literal "false", keep plaintext refused without stopping the +// process. An empty value is "not asserted" rather than a misspelling. +func TestMeshProtectedTreatsUnsetAndFalseAsNotAsserted(t *testing.T) { + for name, source := range map[string]transport{ + "nothing carries it": {}, + "an empty value": {assertion: "", set: true}, + "false": {assertion: "false", set: true}, + } { + t.Run(name, func(t *testing.T) { + protected, err := MeshProtected(source) + + require.NoError(t, err) + require.False(t, protected) + }) + } +} + +// A LOOKUP FAILURE IS NOT ABSENCE. An unreadable delivery, or an +// authority-bearing value that has drifted since boot, is a refusal — reading it +// as "not asserted" is a failed read quietly deciding a posture question. +func TestMeshProtectedRefusesALookupFailureRatherThanReadingItAsAbsence(t *testing.T) { + const marker = "zzsecretzz" + failure := fmt.Errorf("the pinned value changed from %q", marker) + + _, err := MeshProtected(transport{failure: failure}) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorIs(t, err, failure, "the source's error stays in the chain for errors.Is") + require.ErrorContains(t, err, "could not be read") + require.NotContains(t, err.Error(), marker, "the refusal echoes the value the lookup named") +} + +// And the failure reaches every admission decision, including an https one: a +// declaration that cannot be read must not leave the process admitting. +func TestALookupFailureRefusesEveryKeySetEndpoint(t *testing.T) { + source := transport{failure: errors.New("unreadable")} + for _, raw := range []string{ + "https://authority.example/keys", + "http://authority.platform.svc/keys", + "http://127.0.0.1:8080/keys", + } { + _, err := ResolveKeySetEndpoint(raw, source) + + require.ErrorIs(t, err, ErrInvalid, "admitting %q after a failed declaration read", raw) + require.ErrorContains(t, err, "could not be read") + } +} + +func TestResolveKeySetEndpointNeedsATransportSource(t *testing.T) { + _, err := ResolveKeySetEndpoint("https://authority.example/keys", nil) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "needs a transport source") +} + +// The assertion is read whatever the URL's scheme, so a misspelling cannot sit +// unread behind an https URL until the day somebody changes one. +func TestAMisspelledAssertionIsRefusedEvenForAnHTTPSKeySet(t *testing.T) { + _, err := ResolveKeySetEndpoint("https://authority.example/keys", transport{assertion: "True", set: true}) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, MeshTransportKey) +} + +// --------------------------------------------------------------------------- +// The address rule. +// --------------------------------------------------------------------------- + +func TestResolveKeySetEndpointAdmitsOnlyTheMeshContract(t *testing.T) { + accepted := map[string]struct { + url string + source TransportSource + }{ + "https to any host": {"https://authority.example/keys", unasserted}, + "https with a port and a path": {"https://authority.example:8443/.well-known/keys", unasserted}, + "https needs no assertion": {"https://authority.svc.cluster.local/keys", unasserted}, + "https is admitted with the assertion": {"https://authority.example/keys", meshed}, + "plaintext to an in-cluster Service": {"http://authority.platform.svc/keys", meshed}, + "plaintext with the cluster domain": {"http://authority.platform.svc.cluster.local/keys", meshed}, + "plaintext in-cluster with a port": {"http://authority.platform.svc:8080/keys", meshed}, + "a trailing dot on the cluster domain": {"http://authority.platform.svc.cluster.local./keys", meshed}, + "labels with digits and hyphens": {"http://auth-1.platform-2.svc/keys", meshed}, + "a loopback name on a local run": {"http://localhost:8080/keys", localRun}, + "a loopback literal on a local run": {"http://127.0.0.1:8080/keys", localRun}, + "a loopback v6 literal on a local run": {"http://[::1]:8080/keys", localRun}, + "an uppercase loopback name locally": {"http://LOCALHOST:8080/keys", localRun}, + "an uppercase scheme is still https": {"HTTPS://authority.example/keys", unasserted}, + } + for name, test := range accepted { + t.Run("accepts "+name, func(t *testing.T) { + endpoint, err := ResolveKeySetEndpoint(test.url, test.source) + + require.NoError(t, err) + require.NotEmpty(t, endpoint.URL()) + require.Equal(t, endpoint.URL(), endpoint.String()) + }) + } + + refused := map[string]struct { + url string + source TransportSource + names string + }{ + // Neither half of the contract admits on its own. + "an in-cluster Service without the assertion": { + "http://authority.platform.svc/keys", unasserted, "is not asserted", + }, + "an in-cluster Service with the assertion false": { + "http://authority.platform.svc/keys", transport{assertion: "false", set: true}, "is not asserted", + }, + "an external host with the assertion set": { + "http://authority.example/keys", meshed, "not an in-cluster Service address", + }, + // The suffix is matched literally. + "an svc label in another domain": { + "http://authority.platform.svc.example.com/keys", meshed, "not an in-cluster Service address", + }, + "the cluster domain with something after it": { + "http://authority.platform.svc.cluster.local.example.com/keys", meshed, + "not an in-cluster Service address", + }, + "an extra prefix label": { + "http://extra.authority.platform.svc/keys", meshed, "not an in-cluster Service address", + }, + "only two labels": {"http://authority.svc/keys", meshed, "not an in-cluster Service address"}, + "four labels": {"http://authority.platform.svc.cluster/keys", meshed, "not an in-cluster Service address"}, + "svc in the wrong position": { + "http://svc.authority.platform/keys", meshed, "not an in-cluster Service address", + }, + "a bare IP with the assertion set": { + "http://10.4.1.9:8080/keys", meshed, "not an in-cluster Service address", + }, + "a short name with the assertion set": { + "http://authority/keys", meshed, "not an in-cluster Service address", + }, + // DNS-1123 on both labels before svc. + "an empty service label": {"http://.platform.svc/keys", meshed, "not an in-cluster Service"}, + "an empty namespace label": {"http://authority..svc/keys", meshed, "not an in-cluster Service"}, + "a leading hyphen": {"http://-authority.platform.svc/keys", meshed, "not an in-cluster Service"}, + "a trailing hyphen": {"http://authority-.platform.svc/keys", meshed, "not an in-cluster Service"}, + "an underscore": {"http://authority_1.platform.svc/keys", meshed, "not an in-cluster Service"}, + "a label of sixty-four characters": {"http://" + strings.Repeat("a", 64) + ".platform.svc/keys", meshed, "not an in-cluster Service"}, + // Loopback is the local shape only. + "loopback off a local run": { + "http://127.0.0.1:8080/keys", unasserted, "not a local run", + }, + "loopback off a local run with the assertion set": { + "http://127.0.0.1:8080/keys", meshed, "not a local run", + }, + "a loopback name off a local run": { + "http://localhost:8080/keys", meshed, "not a local run", + }, + // A name that merely contains a loopback spelling is a different host. + "a host containing localhost": {"http://localhost.example.invalid/keys", meshedLocal, "not an in-cluster Service"}, + "a host containing a v4 loopback": {"http://127.0.0.1.example.invalid/keys", meshedLocal, "not an in-cluster Service"}, + "notlocalhost": {"http://notlocalhost/keys", meshedLocal, "not an in-cluster Service"}, + // Not http and not https. + "ftp": {"ftp://authority.example/keys", unasserted, "scheme is not https"}, + "ws": {"ws://authority.example/keys", unasserted, "scheme is not https"}, + "scheme relative": {"//authority.example/keys", unasserted, "scheme is not https"}, + // No host to authenticate. + "a file URL": {"file:///etc/keys.json", unasserted, "absolute URL naming a host"}, + "a port with no host": {"https://:443/keys", unasserted, "absolute URL naming a host"}, + "a path only": {"/well-known/keys", unasserted, "absolute URL naming a host"}, + "a bare host": {"authority.example/keys", unasserted, "absolute URL naming a host"}, + "an opaque URL": {"mailto:keys@example.test", unasserted, "absolute URL naming a host"}, + // Malformed: refused, and NOT a panic. + "a malformed port": {"https://authority.example:ht/keys", unasserted, "absolute URL naming a host"}, + "a malformed escape": {"https://authority.example/%zz", unasserted, "absolute URL naming a host"}, + "an unclosed bracket": {"https://[::1/keys", unasserted, "absolute URL naming a host"}, + "a control character": {"https://authority.example/\x7f", unasserted, "absolute URL naming a host"}, + // A value that leaks into a log or carries a credential. + "userinfo": {"https://user:pass@authority.example/keys", unasserted, "no credentials, query or fragment"}, + "a query": {"https://authority.example/keys?token=abc", unasserted, "no credentials, query or fragment"}, + "an empty query": {"https://authority.example/keys?", unasserted, "no credentials, query or fragment"}, + "a fragment": {"https://authority.example/keys#frag", unasserted, "no credentials, query or fragment"}, + // Unset. + "empty": {"", unasserted, "the key set is unset"}, + "whitespace": {" ", unasserted, "the key set is unset"}, + } + for name, test := range refused { + t.Run("refuses "+name, func(t *testing.T) { + endpoint, err := ResolveKeySetEndpoint(test.url, test.source) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, test.names) + require.Empty(t, endpoint.URL()) + }) + } +} + +func TestEveryPlaintextRefusalCarriesTheRemedy(t *testing.T) { + for _, test := range []struct { + url string + source TransportSource + }{ + {"http://authority.platform.svc/keys", unasserted}, + {"http://authority.example/keys", meshed}, + {"http://127.0.0.1/keys", meshed}, + {"ftp://authority.example/keys", unasserted}, + } { + _, err := ResolveKeySetEndpoint(test.url, test.source) + + require.ErrorContains(t, err, MeshTransportRemedy, "refusing %q", test.url) + } +} + +// No refusal echoes any part of the URL, the parser's own message included. +func TestNoRefusalEchoesTheURL(t *testing.T) { + const marker = "zzmarkerzz" + for _, raw := range []string{ + "https://user:" + marker + "@authority.example/keys", + "https://authority.example/keys?token=" + marker, + "https://authority.example/keys#" + marker, + "https://authority.example:" + marker + "/keys", + "https://authority.example/%zz" + marker, + "https://[::1" + marker + "/keys", + "ftp://" + marker + ".example/keys", + "http://" + marker + ".example/keys", + "http://127.0.0.1/" + marker, + marker + "/keys", + } { + _, err := ResolveKeySetEndpoint(raw, meshed) + + require.Error(t, err, "%q must be refused", raw) + require.NotContains(t, err.Error(), marker, "the refusal for %q echoes part of the URL", raw) + } +} + +func TestKeySetEndpointCarriesTheAdmittedURL(t *testing.T) { + endpoint, err := ResolveKeySetEndpoint(" https://authority.example/keys ", unasserted) + require.NoError(t, err) + require.Equal(t, "https://authority.example/keys", endpoint.URL()) + + require.Empty(t, KeySetEndpoint{}.URL(), "a zero value names no location") + require.Empty(t, KeySetEndpoint{}.String()) +} + +// --------------------------------------------------------------------------- +// Acquisition: the keys a verifier holds are the keys an admitted endpoint served. +// --------------------------------------------------------------------------- + +func TestAcquireKeySetFetchesFromTheAdmittedEndpoint(t *testing.T) { + key := mustKey(t) + endpoint := servedKeySet(t, keySetLine(testKeyID, key)) + + set, err := AcquireKeySet(t.Context(), endpoint, decodeKeySet) + + require.NoError(t, err) + require.Equal(t, endpoint.URL(), set.Endpoint()) + require.Equal(t, []string{testKeyID}, set.KeyIDs()) +} + +func TestAcquireKeySetRefusesWhatItCannotUse(t *testing.T) { + t.Run("a zero endpoint", func(t *testing.T) { + _, err := AcquireKeySet(t.Context(), KeySetEndpoint{}, decodeKeySet) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "needs an admitted endpoint") + }) + + t.Run("no decoder", func(t *testing.T) { + _, err := AcquireKeySet(t.Context(), servedKeySet(t, ""), nil) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "needs a decoder") + }) + + t.Run("a redirect, which leaves the admitted address", func(t *testing.T) { + elsewhere := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(keySetLine(testKeyID, mustKey(t)))) + })) + t.Cleanup(elsewhere.Close) + endpoint := servedBy(t, func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, elsewhere.URL, http.StatusFound) + }) + + _, err := AcquireKeySet(t.Context(), endpoint, decodeKeySet) + + require.ErrorIs(t, err, ErrKeySetUnavailable) + require.ErrorContains(t, err, "redirected") + }) + + t.Run("a status", func(t *testing.T) { + endpoint := servedBy(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + }) + + _, err := AcquireKeySet(t.Context(), endpoint, decodeKeySet) + + require.ErrorIs(t, err, ErrKeySetUnavailable) + require.ErrorContains(t, err, "503") + }) + + t.Run("a body larger than this module reads", func(t *testing.T) { + endpoint := servedBy(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(make([]byte, maxKeySetBytes+1)) + }) + + _, err := AcquireKeySet(t.Context(), endpoint, decodeKeySet) + + require.ErrorIs(t, err, ErrKeySetUnavailable) + require.ErrorContains(t, err, "larger than") + }) + + t.Run("an endpoint that does not answer", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + endpoint, err := ResolveKeySetEndpoint(server.URL+"/keys", localRun) + require.NoError(t, err) + server.Close() + + _, err = AcquireKeySet(t.Context(), endpoint, decodeKeySet) + + require.ErrorIs(t, err, ErrKeySetUnavailable) + }) + + t.Run("a decoder error, whose text is the caller's", func(t *testing.T) { + const marker = "zzdecoderzz" + endpoint := servedKeySet(t, "whatever") + + _, err := AcquireKeySet(t.Context(), endpoint, func([]byte) (map[string][]byte, error) { + return nil, errors.New(marker) + }) + + require.ErrorIs(t, err, ErrInvalid) + require.NotContains(t, err.Error(), marker, "a decoder's text may name the bytes it read") + }) + + t.Run("no key at all", func(t *testing.T) { + endpoint := servedKeySet(t, "") + + _, err := AcquireKeySet(t.Context(), endpoint, func([]byte) (map[string][]byte, error) { + return map[string][]byte{}, nil + }) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "served no key") + }) + + t.Run("more keys than a verifier holds", func(t *testing.T) { + lines := make([]string, 0, maxKeySetKeys+1) + for index := range maxKeySetKeys + 1 { + lines = append(lines, keySetLine(fmt.Sprintf("key-%d", index), mustKey(t))) + } + + _, err := AcquireKeySet(t.Context(), servedKeySet(t, strings.Join(lines, "\n")), decodeKeySet) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "at most") + }) + + t.Run("a key with no id", func(t *testing.T) { + endpoint := servedKeySet(t, "whatever") + + _, err := AcquireKeySet(t.Context(), endpoint, func([]byte) (map[string][]byte, error) { + return map[string][]byte{"": make([]byte, verificationKeySize)}, nil + }) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "no id") + }) + + t.Run("no context", func(t *testing.T) { + //nolint:staticcheck // a nil context is exactly what this refuses. + _, err := AcquireKeySet(nil, servedKeySet(t, ""), decodeKeySet) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "needs a context") + }) +} + +// --------------------------------------------------------------------------- +// Settling the verifier. +// --------------------------------------------------------------------------- + +func TestNewVerifierRefusesAnIncompleteConfigurationByName(t *testing.T) { + cases := map[string]struct { + settings func(VerifierSettings) VerifierSettings + keys func(*testing.T) KeySet + names string + }{ + "no issuer": { + settings: func(s VerifierSettings) VerifierSettings { s.Issuer = ""; return s }, + names: "no issuer is pinned", + }, + "no audience": { + settings: func(s VerifierSettings) VerifierSettings { s.Audience = ""; return s }, + names: "no audience is pinned", + }, + "issuer carrying whitespace": { + settings: func(s VerifierSettings) VerifierSettings { s.Issuer = " " + testIssuer; return s }, + names: "issuer carries surrounding whitespace", + }, + "audience carrying whitespace": { + settings: func(s VerifierSettings) VerifierSettings { s.Audience = testAudience + "\n"; return s }, + names: "audience carries surrounding whitespace", + }, + "a key set that was never acquired": { + keys: func(*testing.T) KeySet { return KeySet{} }, + names: "no key set from an admitted endpoint", + }, + "no revision source": { + settings: func(s VerifierSettings) VerifierSettings { s.Revisions = nil; return s }, + names: "no revision source", + }, + "no replay store": { + settings: func(s VerifierSettings) VerifierSettings { s.Replay = nil; return s }, + names: "no replay store", + }, + "no grant source": { + settings: func(s VerifierSettings) VerifierSettings { s.Grants = nil; return s }, + names: "no grant source", + }, + "no seal source": { + settings: func(s VerifierSettings) VerifierSettings { s.Seals = nil; return s }, + names: "no seal source", + }, + } + for name, test := range cases { + t.Run(name, func(t *testing.T) { + a := newAuthority(t) + settings := settingsFor(a) + if test.settings != nil { + settings = test.settings(settings) + } + keys := acquired(t, mustKey(t)) + if test.keys != nil { + keys = test.keys(t) + } + + pinned, err := NewVerifier(settings, keys) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, test.names) + require.Nil(t, pinned, "a refused configuration hands back nothing to verify with") + }) + } +} + +// Key material of the wrong length is refused when the verifier is settled, +// naming the key id. Core refuses it per request; a configuration fault belongs +// in a refusal before anything is served. +func TestNewVerifierRefusesUnusableKeyMaterial(t *testing.T) { + for _, size := range []int{0, 1, 16, 31, 33, 64} { + t.Run(fmt.Sprintf("%d bytes", size), func(t *testing.T) { + set, err := AcquireKeySet(t.Context(), servedKeySet(t, "whatever"), + func([]byte) (map[string][]byte, error) { + return map[string][]byte{testKeyID: make([]byte, size)}, nil + }) + require.NoError(t, err) + + pinned, err := NewVerifier(settingsFor(newAuthority(t)), set) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, fmt.Sprintf("verification key %q is %d bytes", testKeyID, size)) + require.Nil(t, pinned) + }) + } + + // The id named is stable across runs: two unusable entries must not produce + // a different message each time. + t.Run("two unusable keys name the first id in order", func(t *testing.T) { + set, err := AcquireKeySet(t.Context(), servedKeySet(t, "whatever"), + func([]byte) (map[string][]byte, error) { + return map[string][]byte{"key-z": make([]byte, 7), "key-a": make([]byte, 9)}, nil + }) + require.NoError(t, err) + + for range 20 { + _, err := NewVerifier(settingsFor(newAuthority(t)), set) + require.ErrorContains(t, err, `verification key "key-a" is 9 bytes`) + } + }) +} + +// The whole path, end to end: admit the endpoint, fetch the keys from it, settle +// the verifier, verify a capability the authority minted. +func TestAVerifierSettledFromAnAdmittedEndpointVerifies(t *testing.T) { + signing := newSigningAuthority(t) + + pinned, err := NewVerifier(settingsFor(signing.authority), acquired(t, signing.public)) + require.NoError(t, err) + require.Equal(t, testIssuer, pinned.Issuer()) + require.Equal(t, testAudience, pinned.Audience()) + require.NotEmpty(t, pinned.KeySetEndpoint()) + + token := signing.startAt(t, signing.now(), mintInput{}, 15*time.Minute) + + verified, err := pinned.Verify(t.Context(), token) + + require.NoError(t, err) + require.Equal(t, testAudience, verified.Context().GetAudience()) + require.NoError(t, pinned.Recheck(t.Context(), verified)) +} + +// A verifier accepts a capability only when it carries the configured issuer. +func TestASettledVerifierRefusesAnotherIssuer(t *testing.T) { + signing := newSigningAuthority(t) + signing.core.Issuer = "codefly.other-authority" + token := signing.startAt(t, signing.now(), mintInput{}, 15*time.Minute) + + pinned, err := NewVerifier(settingsFor(signing.authority), acquired(t, signing.public)) + require.NoError(t, err) + + _, err = pinned.Verify(t.Context(), token) + + require.ErrorIs(t, err, ErrInvalid) + require.ErrorContains(t, err, "codefly.other-authority") +} + +// A verifier holds the keys the endpoint served and nothing else: a capability +// signed under a key that endpoint did not serve is refused. +func TestAVerifierHoldsOnlyTheKeysItsEndpointServed(t *testing.T) { + signing := newSigningAuthority(t) + token := signing.startAt(t, signing.now(), mintInput{}, 15*time.Minute) + + // An endpoint serving a DIFFERENT key under the same id. + pinned, err := NewVerifier(settingsFor(signing.authority), acquired(t, mustKey(t))) + require.NoError(t, err) + + _, err = pinned.Verify(t.Context(), token) + + require.ErrorIs(t, err, ErrInvalid) +} + +func TestANilPinnedVerifierRefuses(t *testing.T) { + var pinned *PinnedVerifier + + _, err := pinned.Verify(t.Context(), "") + require.ErrorIs(t, err, ErrInvalid) + require.ErrorIs(t, pinned.Recheck(t.Context(), nil), ErrInvalid) +} + +// Nothing in the settings permits plaintext or an unset issuer. The shape is +// frozen by an UNKEYED literal, which stops compiling the moment a field is +// added or reordered, so a field admitting something on its own say-so could not +// arrive unnoticed. +func TestNothingInTheVerifierSettingsTurnsACheckOff(t *testing.T) { + a := newAuthority(t) + settings := VerifierSettings{ + testIssuer, testAudience, + corework.FixedRevision(5), corework.NewMemoryReplayStore(), noGrants{}, a.seals, + a.now, 0, + } + + pinned, err := NewVerifier(settings, acquired(t, mustKey(t))) + + require.NoError(t, err) + require.Equal(t, testIssuer, pinned.Issuer()) +} + +// --------------------------------------------------------------------------- +// The gate: there is no verification path here that was not settled. +// --------------------------------------------------------------------------- + +// THE CHECK SP-WC-05 IS RE-AUDITED BY, in this repository. +// +// A verifier whose issuer, audience and key set are required is only a +// requirement if there is no second way to get one. Core's verifier is a struct +// whose every field has a usable zero value, so re-exporting it was a path that +// skipped every check in this file — the same shape GuardStreams exists to +// avoid, where the correct wiring took two calls and only one was reachable from +// the type system. +// +// So: nothing this package EXPORTS may name core's Verifier, and the only +// exported Verify and Recheck methods are PinnedVerifier's. Violate either and +// this test fails, which is what makes it a check rather than a description. +func TestEveryVerificationPathIsPinned(t *testing.T) { + const coreVerifier = coreModulePath + "/workcontext#Verifier" + var findings []string + + for _, file := range repositoryFiles(t) { + if filepath.Dir(file.path) != "workcontext" || strings.HasSuffix(file.path, "_test.go") { + continue + } + for _, declaration := range file.syntax.Decls { + switch declared := declaration.(type) { + case *ast.GenDecl: + for _, spec := range declared.Specs { + typed, ok := spec.(*ast.TypeSpec) + if !ok || !typed.Name.IsExported() { + continue + } + if typeName(file, typed.Type) == coreVerifier { + findings = append(findings, fmt.Sprintf( + "%s exports %s, which names core's Verifier.\n"+ + "Core's verifier is constructible with every check unset, so exporting it "+ + "is a second way in that skips the configuration NewVerifier requires.", + file.path, typed.Name.Name)) + } + } + case *ast.FuncDecl: + if !declared.Name.IsExported() { + continue + } + if receiver := receiverTypeName(declared); receiver != "" { + if (declared.Name.Name == "Verify" || declared.Name.Name == "Recheck") && + receiver != "PinnedVerifier" { + findings = append(findings, fmt.Sprintf( + "%s declares (%s).%s.\n"+ + "PinnedVerifier is the only type here that verifies, because it is the only "+ + "one that cannot exist without an issuer, an audience and an acquired key set.", + file.path, receiver, declared.Name.Name)) + } + continue + } + if declared.Type.Results == nil { + continue + } + for _, result := range declared.Type.Results.List { + if typeName(file, result.Type) == coreVerifier { + findings = append(findings, fmt.Sprintf( + "%s exports func %s returning core's Verifier.\n"+ + "A caller holding one can verify without any of the configuration this "+ + "package requires.", + file.path, declared.Name.Name)) + } + } + } + } + } + + require.Empty(t, findings, "%s", strings.Join(findings, "\n\n")) +} + +// receiverTypeName is the bare type name a method is declared on, through a +// pointer, and "" for a function. +func receiverTypeName(declared *ast.FuncDecl) string { + if declared.Recv == nil || len(declared.Recv.List) == 0 { + return "" + } + expression := declared.Recv.List[0].Type + if star, ok := expression.(*ast.StarExpr); ok { + expression = star.X + } + if identifier, ok := expression.(*ast.Ident); ok { + return identifier.Name + } + return "" +}