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 "" +}