diff --git a/.github/wiki/guides/ci/Trust-Security-and-Privacy.md b/.github/wiki/guides/ci/Trust-Security-and-Privacy.md index 3adaaf8bb..9f0f99161 100644 --- a/.github/wiki/guides/ci/Trust-Security-and-Privacy.md +++ b/.github/wiki/guides/ci/Trust-Security-and-Privacy.md @@ -402,7 +402,7 @@ Without a known-good backup and its KEK, this procedure cannot recover the chain What the chain does not give you: it detects an edit that was not recomputed. Someone who can write the database and recompute every later row defeats it; the TEAM checkpoint detects that (see Signed evidence, revocation and the audit checkpoint). A PERSONAL coordinator has neither a checkpoint nor a second writer to guard against, so the chain there is a tidy record, not evidence. Someone who can write the database can also clear heads and let the next `Append` re-chain them; that counts as a database writer too. (Removing the newest rows is caught by the seq check above, but only until the counter itself is edited.) -Handoff for later Tickets: `roles.Authorize(ctx, actor, role, project)` and `roles.AuthorizeAction(ctx, actor, action, project)` are the calls a route makes; `WithAuthor(ctx, id)` carries a run's author for `run.approve`. Both write one `authorize` row per release- or admin-level decision (allowed or refused, an unknown role included) and none for member-level reads, so a mutation that writes its own row records two rows for one request. The `serve admin` commands write only their own mutation row and do not audit failed validation, failed token or certificate sign-in, or the local operator's own reads. The non-author rule for `run.approve` compares principal ids (an audit label such as `token:tok-1:bob` names `bob`); the local operator is exempt only on a PERSONAL coordinator, where it is the only person; on a TEAM coordinator it is refused as `self_approval` for a run it authored. +Handoff for later Tickets: `roles.Authorize(ctx, actor, role, project)` and `roles.AuthorizeAction(ctx, actor, action, project)` are the calls a route makes; `WithAuthor(ctx, id)` carries a run's author for `run.approve`. Both write one `authorize` row per release- or admin-level decision (allowed or refused, an unknown role included) and none for member-level reads, so a mutation that writes its own row records two rows for one request. That two-row pattern is the design: the `authorize` row is the decision and exists whether or not the mutation follows (a refusal never reaches a mutation row), and the mutation row is written first and fails closed. A refused token or certificate sign-in writes a `signin.fail` row (`token:unknown`, `token::`, `user:` or `certificate:unknown` as the actor, the reason key and the peer address in the detail, never the token). It is rate limited: one row per peer and kind per 30 seconds, at most 20 rows per 30 seconds across all peers, and the next row says how many were held back (`suppressed=N`). A refused `serve admin` mutation (a validation or role refusal) writes one `.refused` row (`roles.grant.refused`, `roles.revoke.refused`, `token.issue.refused`, `token.revoke.refused`) with the reason key. The `serve admin` commands otherwise write only their own mutation row and do not audit the local operator's own reads. The non-author rule for `run.approve` compares principal ids (an audit label such as `token:tok-1:bob` names `bob`); the local operator is exempt only on a PERSONAL coordinator, where it is the only person; on a TEAM coordinator it is refused as `self_approval` for a run it authored. Wiring left to the coordinator process (P7-NODE-10 and P7-TRUST-26): start `audit.Log.RunSealer` (60 s) so rows other packages write get sealed, call `audit.SetScanner(run.Redactions)` before its first `Append` or `AppendTx` (the `serve admin` commands do this themselves), and resolve an Actor per request, not per connection. `store.CertSubjectPut` has no caller until the command that issues user certificates records their subject. @@ -460,6 +460,10 @@ A node that is signing eligible signs the evidence of every job it runs. The sig A node signs only if its agent state directory holds an empty file named `signing-eligible` (a regular file, not a symlink, not writable by group or other). Create it, or let provisioning create it, only on a node whose jobs cannot read the node key: a separate job UID, a user namespace or a container. Without the file results carry no signature and count as unsigned. The coordinator's `signing_eligible` fact decides regardless of the file. A node key that was rotated away no longer verifies old evidence; a revoked key stays revoked (the id is derived from the key). +The coordinator also checks the signature of a result frame before it settles the attempt from the frame. A frame that carries a signature the registry does not verify (unknown, revoked or wrong key, a node that did not hold the lease, a node that is not signing eligible, a replay) ends the attempt `lost`, which counts against the node and takes the infra retry; it never settles as a pass. An unsigned frame is still accepted, because a node without the marker file completes unsigned, and the release gate refuses unsigned remote evidence. A result a node reports as an infra failure, and a cancel the run did not ask for (a drain or an agent self-cancel), also end the attempt `lost` and take the infra retry; a cancel the run asked for settles as cancelled. + +The runner and coordinator ids in public provenance, evidence and statuses are pseudonyms: `runner-` or `coordinator-` and 12 hex characters of an HMAC-SHA256 under a secret the operator holds. The secret is 32 random bytes in `pseudonym.key` in the CI home (mode 0600, never a link), created by the first attempt or the first `nself ci artifacts sign` and never published; a wrong-sized, group-readable or non-regular file fails the attempt or the signing instead of falling back to an unkeyed hash. A dictionary of host names cannot recover a builder id without it. Deleting the file gives every id a new pseudonym. + **Revocation.** `run.RevokeNode` (and `attest.Revoker`) revoke every evidence key a node pinned or ever used, in one transaction, and quarantine the runs those keys signed. The call returns the gates, posted statuses and promoted releases the revoked evidence supported. It writes an `evidence.revoke` audit event and one `release.quarantine` event per promoted release, and re-posts each affected status as `error` with the reason `node.revoked`. `nself ci artifacts verify` reports a quarantined release as `quarantined` and exits 1; it reads the quarantine index from the coordinator store, so on a machine without one it cannot know and reports nothing. A node revoked with `nodes revoke` (registry state `revoked`) or by `RevokeNode` no longer verifies new evidence; a key pinned for it later is refused too. **Audit checkpoint (TEAM only).** The audit chain detects an edit that was not recomputed. A signed checkpoint detects a database writer who recomputed every row. diff --git a/free/ci/cmd/serve_v15.go b/free/ci/cmd/serve_v15.go index 91a3062f1..11f5a7c74 100644 --- a/free/ci/cmd/serve_v15.go +++ b/free/ci/cmd/serve_v15.go @@ -176,6 +176,7 @@ type pipelineFacts struct { Trigger string `json:"trigger"` CloneURL string `json:"clone_url"` Synthetic json.RawMessage `json:"synthetic"` + Submitter string `json:"submitter"` // the audit label of the user who submitted the run, when one did } // dispatchPipeline runs one queued trigger: clone the revision into a fresh directory, plan it and run the @@ -206,6 +207,7 @@ func dispatchPipeline(home string, f *v15ServeFlags) coordinator.Dispatcher { } o := RunOptions{Root: dir, Home: home, Pipeline: row.Pipeline, Trigger: serveTrigger(facts.Trigger), Container: !f.unsandboxed, ProviderRepo: providerRepoFor(row.Source, row.Project), Timeout: time.Duration(f.timeout) * time.Second, Stream: f.verbose} + o.User, o.Team = submitterIdentity(facts.Submitter, row.Project) p, digest, subst, err := preparePlan(ctx, o) if err != nil { return coordinator.RunResult{}, fmt.Errorf("plan failed: %w", err) diff --git a/free/ci/cmd/serve_v15_identity.go b/free/ci/cmd/serve_v15_identity.go new file mode 100644 index 000000000..6e7467987 --- /dev/null +++ b/free/ci/cmd/serve_v15_identity.go @@ -0,0 +1,25 @@ +package main + +// Purpose: the user and team a served run counts against in the scheduler limits (D-1044). +// Inputs: the audit label the user API stored as the submitter of a run, and the run's project. +// Outputs: the principal id (user) and the owner of the project (team). +// Constraints: a run nobody submitted through the user API (a webhook, a provider delivery) has no user, +// and its team is still the project owner; an empty user and team keep the shared bucket. + +import ( + "strings" + + "github.com/nself-org/plugins/free/ci/internal/roles" +) + +// submitterIdentity returns the user and team of a run. The team is the owner part of owner/repo: the +// coordinator holds no team table, and the project owner is the organisation the repository belongs to. +func submitterIdentity(submitter, project string) (user, team string) { + if strings.TrimSpace(submitter) != "" { + user = roles.PrincipalOf(submitter) + } + if owner, _, ok := strings.Cut(project, "/"); ok { + team = owner + } + return user, team +} diff --git a/free/ci/cmd/serve_v15_identity_test.go b/free/ci/cmd/serve_v15_identity_test.go new file mode 100644 index 000000000..32ab2b08b --- /dev/null +++ b/free/ci/cmd/serve_v15_identity_test.go @@ -0,0 +1,17 @@ +package main + +import "testing" + +func TestSubmitterIdentity(t *testing.T) { + for _, c := range []struct{ submitter, project, user, team string }{ + {"user:Bob role=member", "acme/web", "bob", "acme"}, + {"token:tok-1:mia role=admin", "acme/web", "mia", "acme"}, + {"operator:cli:ali role=owner", "o/r", "ali", "o"}, + {"", "acme/web", "", "acme"}, + {"user:bob role=member", "noslash", "bob", ""}, + } { + if u, tm := submitterIdentity(c.submitter, c.project); u != c.user || tm != c.team { + t.Errorf("submitterIdentity(%q, %q) = %q, %q; want %q, %q", c.submitter, c.project, u, tm, c.user, c.team) + } + } +} diff --git a/free/ci/internal/attest/frame.go b/free/ci/internal/attest/frame.go new file mode 100644 index 000000000..798afc989 --- /dev/null +++ b/free/ci/internal/attest/frame.go @@ -0,0 +1,42 @@ +package attest + +// Purpose: verifies the signature of a result frame (D-1103). The agent signs the frame's evidence under +// the signed-remote payload (SignDocumentRaw) and sends the key id and signature beside it, not inside +// it; the coordinator rebuilds the signed-remote document and checks it with RunnerVerifier, so the key, +// purpose, revocation, lease holder, eligibility and replay rules are the evidence gate's own. +// Inputs: a ledger, the evidence bytes of the frame, and the frame's key id and signature text. +// Outputs: nil, or a *RemoteError whose Reason says why (code E686). +// Constraints: both signature fields are required (an unsigned frame is the caller's policy, not +// decided here); fail closed on every ledger error; an accepted signature is consumed. + +import ( + "encoding/json" + + "github.com/nself-org/plugins/free/ci/internal/model" +) + +// VerifyFrame checks the signature value under keyID over the evidence of a result frame. +func VerifyFrame(led func() Ledger, evidence []byte, keyID, value string) error { + if keyID == "" || value == "" { + return remoteFail(RemoteSignatureMissing, "no runner signature", nil) + } + obj, err := decodeObject(evidence) + if err != nil { + return remoteFail(RemoteSignatureInvalid, "no signing payload: "+err.Error(), nil) + } + producer, _ := obj["producer"].(map[string]any) + if producer == nil { + return remoteFail(RemoteSignatureInvalid, "evidence has no producer", nil) + } + producer["kind"] = "signed-remote" + obj["signature"] = map[string]any{"kind": "runner-ed25519", "key_id": keyID, "value": value} + doc, err := json.Marshal(obj) + if err != nil { + return remoteFail(RemoteSignatureInvalid, "no signing payload: "+err.Error(), nil) + } + var ev model.Evidence + if err = json.Unmarshal(doc, &ev); err != nil { + return remoteFail(RemoteSignatureInvalid, "the evidence does not decode: "+err.Error(), nil) + } + return (&RunnerVerifier{Ledger: led}).VerifyEvidence(doc, ev) +} diff --git a/free/ci/internal/attest/frame_test.go b/free/ci/internal/attest/frame_test.go new file mode 100644 index 000000000..5e5777330 --- /dev/null +++ b/free/ci/internal/attest/frame_test.go @@ -0,0 +1,56 @@ +package attest_test + +// Purpose: the result-frame signature check (D-1103): a frame the node signed the way the agent does +// verifies; a changed document, a wrong signer and a missing signature are each refused with their reason. + +import ( + "encoding/base64" + "testing" + + "github.com/nself-org/plugins/free/ci/internal/attest" +) + +func TestVerifyFrame(t *testing.T) { + ev := attemptEvidence("att-1") + ev.Producer.Kind = "self-reported" // what a node's document says before the coordinator accepts the signature + doc := marshal(t, ev) + sign := func(d []byte, s seedSigner) (string, string) { + raw, err := attest.SignDocumentRaw(d, s) + if err != nil { + t.Fatal(err) + } + return attest.EvidenceKeyID(s.KeyID()), base64.RawStdEncoding.EncodeToString(raw) + } + keyID, value := sign(doc, keyA) + otherKey, otherValue := sign(doc, keyB) + changed := ev + changed.Revision = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + tampered := marshal(t, changed) + for _, c := range []struct { + name string + doc []byte + key, sig string + ledger func(*memLedger) + reason attest.RemoteReason + }{ + {name: "valid", doc: doc, key: keyID, sig: value}, + {name: "changed document", doc: tampered, key: keyID, sig: value, reason: attest.RemoteRevisionMismatch}, + {name: "signed by a node that did not hold the lease", doc: doc, key: otherKey, sig: otherValue, reason: attest.RemoteLeaseHolderMismatch}, + {name: "revoked key", doc: doc, key: keyID, sig: value, ledger: func(l *memLedger) { l.revoked[keyID] = true }, reason: attest.RemoteKeyRevoked}, + {name: "no key id", doc: doc, sig: value, reason: attest.RemoteSignatureMissing}, + {name: "no signature", doc: doc, key: keyID, reason: attest.RemoteSignatureMissing}, + {name: "not base64", doc: doc, key: keyID, sig: "not base64!", reason: attest.RemoteSignatureMalformed}, + } { + l := newLedger() + if c.ledger != nil { + c.ledger(l) + } + err := attest.VerifyFrame(func() attest.Ledger { return l }, c.doc, c.key, c.sig) + if got := attest.RemoteReasonOf(err); got != c.reason { + t.Errorf("%s: reason %q (%v), want %q", c.name, got, err, c.reason) + } + } + if err := attest.VerifyFrame(func() attest.Ledger { return nil }, doc, keyID, value); attest.RemoteReasonOf(err) != attest.RemoteLedgerUnavailable { + t.Errorf("no ledger: %v, want ledger_unavailable", err) + } +} diff --git a/free/ci/internal/attest/profile.go b/free/ci/internal/attest/profile.go index c7572ab40..1eacbe84f 100644 --- a/free/ci/internal/attest/profile.go +++ b/free/ci/internal/attest/profile.go @@ -8,7 +8,9 @@ package attest // Outputs: Profile with RunnerID, CoordinatorID, Public, Text and URL. // Constraints: pure; fail closed: only the exact value Private turns the // profile off, so an empty or unknown visibility scrubs; a zero Profile -// scrubs; pseudonyms are stable for one repo and differ between repos. +// scrubs; pseudonyms are stable for one repo and differ between repos. With an operator key +// registered (UsePseudonymKey) a pseudonym is an HMAC under it, so a dictionary of host names cannot +// recover the id; without one it is the unkeyed hash. import ( "net/netip" @@ -38,6 +40,7 @@ type RepoFacts struct { type Profile struct { private bool repoID string + key []byte // the operator's pseudonym key (pseudonym_key.go); nil keeps the unkeyed form hosts []string users []string aliases map[string]string // literal -> replacement, longest applied first @@ -45,7 +48,7 @@ type Profile struct { // ProfileFor builds the profile of one repository. func ProfileFor(f RepoFacts) Profile { - return Profile{private: f.Visibility == Private, repoID: f.RepoID, + return Profile{private: f.Visibility == Private, repoID: f.RepoID, key: currentPseudonymKey(), hosts: append([]string(nil), f.Hostnames...), users: append([]string(nil), f.Usernames...)} } @@ -62,6 +65,9 @@ func (p Profile) pseudonym(kind, id string) string { if p.private || id == "" { return id } + if len(p.key) > 0 { // keyed: a dictionary of host names cannot recover the id (D-1198) + return kind + "-" + keyedPseudonym(p.key, "nself-ci/"+kind+"/v2\x00"+p.repoID+"\x00"+id) + } return kind + "-" + Digest([]byte("nself-ci/" + kind + "/v1\x00" + p.repoID + "\x00" + id))[:12] } diff --git a/free/ci/internal/attest/pseudonym_key.go b/free/ci/internal/attest/pseudonym_key.go new file mode 100644 index 000000000..3482e73e7 --- /dev/null +++ b/free/ci/internal/attest/pseudonym_key.go @@ -0,0 +1,145 @@ +package attest + +// Purpose: the operator-held secret behind the publication pseudonyms (D-1198). An unkeyed hash of a +// repo id and a host name lets anyone with a dictionary of host names recover the builder id of a +// public provenance statement; an HMAC under a secret the operator keeps does not. +// Inputs: the CI home directory (LoadPseudonymKey) or a key (UsePseudonymKey). +// Outputs: the key a Profile built afterwards uses for RunnerID and CoordinatorID. +// Constraints: the key is 32 random bytes in /pseudonym.key, mode 0600, never a link, created +// on first use; a file of another size, mode or type is an error, never replaced. With no +// key registered a Profile keeps the unkeyed pseudonym (the form the golden statements hold), +// so a caller that publishes must register one. The key never appears in a document or a log. + +import ( + "crypto/hmac" + "crypto/rand" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "runtime" + "sync" + "sync/atomic" +) + +// PseudonymKeyFile is the name of the key file under the CI home. +const PseudonymKeyFile = "pseudonym.key" + +const pseudonymKeyLen = 32 + +var ( + pseudonymKey atomic.Pointer[[]byte] + loadMu sync.Mutex // one creator at a time within the process +) + +// UsePseudonymKey sets the key later Profiles pseudonymize with; nil clears it. A key shorter than 32 +// bytes is refused. +func UsePseudonymKey(key []byte) error { + if key == nil { + pseudonymKey.Store(nil) + return nil + } + if len(key) < pseudonymKeyLen { + return errors.New("attest: the pseudonym key must be at least 32 bytes") + } + k := append([]byte(nil), key...) + pseudonymKey.Store(&k) + return nil +} + +func currentPseudonymKey() []byte { + if k := pseudonymKey.Load(); k != nil { + return *k + } + return nil +} + +// UsePseudonymKeyAt loads (creating it when absent) the key under home and registers it, so a process +// that publishes without running an attempt (`nself ci artifacts sign`) pseudonymizes with the same key +// as a run does. +func UsePseudonymKeyAt(home string) error { + key, err := LoadPseudonymKey(home) + if err != nil { + return err + } + return UsePseudonymKey(key) +} + +// LoadPseudonymKey returns the key under home, creating it when the file is absent. +func LoadPseudonymKey(home string) ([]byte, error) { + if home == "" { + return nil, errors.New("attest: no CI home for the pseudonym key") + } + if err := os.MkdirAll(home, 0o700); err != nil { + return nil, err + } + path := filepath.Join(home, PseudonymKeyFile) + loadMu.Lock() + defer loadMu.Unlock() + key, err := readPseudonymKey(path) + if errors.Is(err, os.ErrNotExist) { + return createPseudonymKey(path) + } + return key, err +} + +func readPseudonymKey(path string) ([]byte, error) { + st, err := os.Lstat(path) + if err != nil { + return nil, err + } + if !st.Mode().IsRegular() || st.Size() != pseudonymKeyLen || runtime.GOOS != "windows" && st.Mode().Perm()&0o077 != 0 { + return nil, fmt.Errorf("attest: %s must be a regular 32-byte file with mode 0600", path) + } + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer func() { _ = f.Close() }() + if fs, serr := f.Stat(); serr != nil || !os.SameFile(st, fs) { // the file the checks judged is the file read + return nil, fmt.Errorf("attest: %s changed while it was read", path) + } + key := make([]byte, pseudonymKeyLen) + if _, err = io.ReadFull(f, key); err != nil { + return nil, err + } + return key, nil +} + +// createPseudonymKey writes the key to a private temporary file and links it into place, so no reader +// ever sees a partly written key file and a concurrent creator loses cleanly. +func createPseudonymKey(path string) ([]byte, error) { + key := make([]byte, pseudonymKeyLen) + if _, err := rand.Read(key); err != nil { + return nil, err + } + tmp, err := os.CreateTemp(filepath.Dir(path), ".pseudonym-*") + if err != nil { + return nil, err + } + defer func() { _ = os.Remove(tmp.Name()) }() + if _, err = tmp.Write(key); err == nil { + err = tmp.Close() + } else { + _ = tmp.Close() + } + if err != nil { + return nil, err + } + if err = os.Link(tmp.Name(), path); errors.Is(err, os.ErrExist) { + return readPseudonymKey(path) // another process created it first + } else if err != nil { + return nil, err + } + return key, nil +} + +// keyedPseudonym is the first 12 hex digits of HMAC-SHA256(key, msg). +func keyedPseudonym(key []byte, msg string) string { + m := hmac.New(sha256.New, key) + _, _ = m.Write([]byte(msg)) + return hex.EncodeToString(m.Sum(nil))[:12] +} diff --git a/free/ci/internal/attest/pseudonym_key_test.go b/free/ci/internal/attest/pseudonym_key_test.go new file mode 100644 index 000000000..0da2067cb --- /dev/null +++ b/free/ci/internal/attest/pseudonym_key_test.go @@ -0,0 +1,101 @@ +package attest_test + +// Purpose: the keyed publication pseudonyms (D-1198): an HMAC under the operator key replaces the +// unkeyed hash, so a dictionary of host names recovers nothing; the key file is created once, private, +// and a wrong file is refused. + +import ( + "bytes" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" + + "github.com/nself-org/plugins/free/ci/internal/attest" +) + +func TestKeyedPseudonymsResistADictionary(t *testing.T) { + t.Cleanup(func() { _ = attest.UsePseudonymKey(nil) }) + facts := attest.RepoFacts{Visibility: attest.Public, RepoID: "local:abc"} + unkeyed := attest.ProfileFor(facts) + if err := attest.UsePseudonymKey(bytes.Repeat([]byte{7}, 32)); err != nil { + t.Fatal(err) + } + keyed := attest.ProfileFor(facts) + // the attacker's dictionary: the unkeyed hash of every host name they can guess + guess := unkeyed.RunnerID("build-host-01") + if got := keyed.RunnerID("build-host-01"); got == guess || !strings.HasPrefix(got, "runner-") || got != keyed.RunnerID("build-host-01") { + t.Fatalf("keyed %q vs dictionary %q", got, guess) + } + if keyed.CoordinatorID("x") == keyed.RunnerID("x") || keyed.RunnerID("x") == attest.ProfileFor(attest.RepoFacts{Visibility: attest.Public, RepoID: "local:other"}).RunnerID("x") { + t.Fatal("pseudonyms must differ by kind and by repo") + } + if err := attest.UsePseudonymKey(bytes.Repeat([]byte{8}, 32)); err != nil { + t.Fatal(err) + } + if attest.ProfileFor(facts).RunnerID("build-host-01") == keyed.RunnerID("build-host-01") { + t.Fatal("another operator key must give another pseudonym") + } + if attest.UsePseudonymKey([]byte("short")) == nil { + t.Fatal("a short key was accepted") + } +} + +func TestLoadPseudonymKeyCreatesOnceAndRefusesAWrongFile(t *testing.T) { + home := filepath.Join(t.TempDir(), "ci") + k1, err := attest.LoadPseudonymKey(home) + if err != nil || len(k1) != 32 { + t.Fatalf("create: %d bytes, %v", len(k1), err) + } + if k2, err := attest.LoadPseudonymKey(home); err != nil || !bytes.Equal(k1, k2) { + t.Fatalf("a second load must return the same key: %v", err) + } + path := filepath.Join(home, attest.PseudonymKeyFile) + if st, _ := os.Stat(path); runtime.GOOS != "windows" && st.Mode().Perm() != 0o600 { + t.Fatalf("mode %v", st.Mode().Perm()) + } + if err = os.WriteFile(path, []byte("short"), 0o600); err != nil { + t.Fatal(err) + } + if _, err = attest.LoadPseudonymKey(home); err == nil { + t.Fatal("a file of the wrong size was accepted") + } + if runtime.GOOS != "windows" { + if err = os.WriteFile(path, bytes.Repeat([]byte{1}, 32), 0o644); err != nil { + t.Fatal(err) + } + if err = os.Chmod(path, 0o644); err != nil { + t.Fatal(err) + } + if _, err = attest.LoadPseudonymKey(home); err == nil { + t.Fatal("a group-readable key file was accepted") + } + } + if _, err = attest.LoadPseudonymKey(""); err == nil { + t.Fatal("an empty home was accepted") + } +} + +// Attempts of one run start together: concurrent first loads agree on one key and none sees a partial file. +func TestLoadPseudonymKeyConcurrentFirstUse(t *testing.T) { + home := t.TempDir() + const n = 16 + keys := make([][]byte, n) + errs := make([]error, n) + var wg sync.WaitGroup + for i := 0; i < n; i++ { + wg.Add(1) + go func() { defer wg.Done(); keys[i], errs[i] = attest.LoadPseudonymKey(home) }() + } + wg.Wait() + for i := range keys { + if errs[i] != nil || !bytes.Equal(keys[i], keys[0]) { + t.Fatalf("load %d: %v", i, errs[i]) + } + } + if left, _ := filepath.Glob(filepath.Join(home, ".pseudonym-*")); len(left) != 0 { + t.Fatalf("temporary files left: %v", left) + } +} diff --git a/free/ci/internal/coordinator/clientapi/auth.go b/free/ci/internal/coordinator/clientapi/auth.go index 03df55d8a..ba1d9da11 100644 --- a/free/ci/internal/coordinator/clientapi/auth.go +++ b/free/ci/internal/coordinator/clientapi/auth.go @@ -11,6 +11,7 @@ package clientapi import ( "errors" + "net" "net/http" "strings" @@ -30,15 +31,16 @@ func (s *Server) authenticate(req *http.Request) (roles.Actor, error) { if req.TLS == nil { return roles.Actor{}, Failf(ReasonPlainHTTP, "the user API serves TLS only") } + ctx := roles.WithPeer(req.Context(), peerHost(req)) if len(req.TLS.PeerCertificates) > 0 { - a, err := s.cfg.Roles.ActorFromPeer(req.Context(), req.TLS.PeerCertificates[0]) + a, err := s.cfg.Roles.ActorFromPeer(ctx, req.TLS.PeerCertificates[0]) return a, mapRoles(err) } tok, ok := bearer(req) if !ok { return roles.Actor{}, Failf(ReasonNoCredential, "present a user certificate or a bearer token") } - a, err := s.cfg.Roles.ActorFromToken(req.Context(), tok) + a, err := s.cfg.Roles.ActorFromToken(ctx, tok) if err != nil { return roles.Actor{}, mapRoles(err) } @@ -48,6 +50,15 @@ func (s *Server) authenticate(req *http.Request) (roles.Actor, error) { return a, nil } +// peerHost is the address the request came from, without the port. +func peerHost(req *http.Request) string { + host, _, err := net.SplitHostPort(req.RemoteAddr) + if err != nil { + return req.RemoteAddr + } + return host +} + // bearer reads the Authorization header; only the Bearer scheme counts. func bearer(req *http.Request) (string, bool) { h := strings.TrimSpace(req.Header.Get("Authorization")) diff --git a/free/ci/internal/release/publish/build.go b/free/ci/internal/release/publish/build.go index ba6d1d4d1..78bbac327 100644 --- a/free/ci/internal/release/publish/build.go +++ b/free/ci/internal/release/publish/build.go @@ -157,6 +157,12 @@ func checksums(f *facts, provenanceSHA string) []byte { // build signs and assembles the release of runID. func build(ctx context.Context, d Deps, runID string, f *facts) (*bundle, error) { + // The runner and coordinator ids published below are an HMAC under the CI home's key (D-1198); this + // path also runs outside a run (`nself ci artifacts sign`), so it registers the key itself and + // refuses to publish the unkeyed, dictionary-recoverable form. + if err := attest.UsePseudonymKeyAt(d.Home); err != nil { + return nil, refuse(signing.ReasonSignerFailed, "the publication pseudonym key is not usable") + } info, err := d.Signer.PublicKey(ctx) if err != nil { return nil, wrapSigner(err) diff --git a/free/ci/internal/release/publish/publish_test.go b/free/ci/internal/release/publish/publish_test.go index d1a94f27b..99d05b55a 100644 --- a/free/ci/internal/release/publish/publish_test.go +++ b/free/ci/internal/release/publish/publish_test.go @@ -2,6 +2,8 @@ package publish_test import ( "context" + "encoding/base64" + "encoding/json" "errors" "os" "path/filepath" @@ -9,6 +11,7 @@ import ( "strings" "testing" + "github.com/nself-org/plugins/free/ci/internal/attest" "github.com/nself-org/plugins/free/ci/internal/model" "github.com/nself-org/plugins/free/ci/internal/release/publish" "github.com/nself-org/plugins/free/ci/internal/signing" @@ -246,3 +249,55 @@ func TestSignSourcesRegistry(t *testing.T) { }() } } + +// TestPublishOutsideARunUsesTheKeyedPseudonyms is the `nself ci artifacts sign` case: a process that +// never ran an attempt has no pseudonym key registered, yet the ids it publishes must be the keyed ones +// (D-1198), the same a run publishes. +func TestPublishOutsideARunUsesTheKeyedPseudonyms(t *testing.T) { + f := newFixture(t) + if err := attest.UsePseudonymKey(nil); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = attest.UsePseudonymKey(nil) }) + unkeyed := attest.ProfileFor(attest.RepoFacts{RepoID: "repo-1"}) + rel := mustRun(t, f) + if _, err := os.Stat(filepath.Join(f.home, attest.PseudonymKeyFile)); err != nil { + t.Fatalf("publishing did not create the key under the CI home: %v", err) + } + keyed := attest.ProfileFor(attest.RepoFacts{RepoID: "repo-1"}) + if keyed.CoordinatorID("local") == unkeyed.CoordinatorID("local") { + t.Fatal("the registered key did not change the pseudonym") + } + for file, id := range map[string]string{ + "release-manifest.dsse.json": unkeyed.CoordinatorID("local"), + "provenance.intoto.dsse.json": unkeyed.RunnerID("local"), + } { + doc := decodePayload(t, filepath.Join(rel.Dir, file)) + if strings.Contains(doc, id) { + t.Fatalf("%s carries the unkeyed id %s", file, id) + } + } + if !strings.Contains(decodePayload(t, filepath.Join(rel.Dir, "release-manifest.dsse.json")), keyed.CoordinatorID("local")) || + !strings.Contains(decodePayload(t, filepath.Join(rel.Dir, "provenance.intoto.dsse.json")), keyed.RunnerID("local")) { + t.Fatal("the published ids are not the keyed ones") + } +} + +func decodePayload(t *testing.T, path string) string { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var env struct { + Payload string `json:"payload"` + } + if err := json.Unmarshal(raw, &env); err != nil { + t.Fatal(err) + } + b, err := base64.StdEncoding.DecodeString(env.Payload) + if err != nil { + t.Fatal(err) + } + return string(b) +} diff --git a/free/ci/internal/release/verify/files.go b/free/ci/internal/release/verify/files.go index fe759759e..f9a514676 100644 --- a/free/ci/internal/release/verify/files.go +++ b/free/ci/internal/release/verify/files.go @@ -66,19 +66,6 @@ func safePath(dir, rel string) (string, error) { return cur, nil } -// open opens the regular file rel under dir. The mode is checked before the -// open, because opening a FIFO would block the verifier for ever. -func open(dir, rel string) (*os.File, error) { - p, err := safePath(dir, rel) - if err != nil { - return nil, err - } - if st, lerr := os.Lstat(p); lerr != nil || !st.Mode().IsRegular() { - return nil, fmt.Errorf("%q is not a regular file", rel) - } - return os.Open(p) -} - // readFile reads rel under dir, refusing more than max bytes. func readFile(dir, rel string, max int64) ([]byte, error) { f, err := open(dir, rel) diff --git a/free/ci/internal/release/verify/open.go b/free/ci/internal/release/verify/open.go new file mode 100644 index 000000000..077d41689 --- /dev/null +++ b/free/ci/internal/release/verify/open.go @@ -0,0 +1,36 @@ +package verify + +// Purpose: open a release file for reading without trusting an earlier look at it (D-1198). +// Inputs: the release directory and a path from the manifest, or an operator trust file path. +// Outputs: an open regular file, or an error. +// Constraints: the Lstat of safePath is a first look only; the open refuses a link and never blocks +// (openRegular), and the mode is read from the opened descriptor. + +import ( + "fmt" + "os" +) + +// open opens the regular file rel under dir. safePath's Lstat is only a first look: a local writer can +// swap the leaf for a symbolic link or a FIFO before the open, so the open itself refuses a link and +// never blocks (openRegular), and the mode is read from the opened descriptor (D-1198). +func open(dir, rel string) (*os.File, error) { + p, err := safePath(dir, rel) + if err != nil { + return nil, err + } + return openChecked(p, rel, false) +} + +// openChecked opens path and refuses anything but a regular file, judged on the descriptor itself. +func openChecked(path, name string, follow bool) (*os.File, error) { + f, err := openRegular(path, follow) + if err != nil { + return nil, err + } + if st, serr := f.Stat(); serr != nil || !st.Mode().IsRegular() { + _ = f.Close() + return nil, fmt.Errorf("%q is not a regular file", name) + } + return f, nil +} diff --git a/free/ci/internal/release/verify/open_other.go b/free/ci/internal/release/verify/open_other.go new file mode 100644 index 000000000..d3b17d2d0 --- /dev/null +++ b/free/ci/internal/release/verify/open_other.go @@ -0,0 +1,11 @@ +//go:build !unix + +package verify + +// Purpose: the open where there is no O_NOFOLLOW or O_NONBLOCK to ask for (D-1198). The caller's +// Lstat of every part and fstat of the opened file are the only checks here. + +import "os" + +// openRegular opens path read-only; follow is not used. +func openRegular(path string, _ bool) (*os.File, error) { return os.Open(path) } diff --git a/free/ci/internal/release/verify/open_unix.go b/free/ci/internal/release/verify/open_unix.go new file mode 100644 index 000000000..4ccfdb9db --- /dev/null +++ b/free/ci/internal/release/verify/open_unix.go @@ -0,0 +1,22 @@ +//go:build unix + +package verify + +// Purpose: the unix half of the release file open (D-1198). +// Constraints: the open itself refuses a symbolic link in the last part (O_NOFOLLOW) and never blocks +// on a FIFO (O_NONBLOCK), so a local writer who swaps a checked file between the check and the open +// gains nothing; the caller still checks the opened descriptor with fstat. + +import ( + "os" + "syscall" +) + +// openRegular opens path read-only. follow allows a symbolic link in the last part. +func openRegular(path string, follow bool) (*os.File, error) { + flags := os.O_RDONLY | syscall.O_NONBLOCK | syscall.O_CLOEXEC + if !follow { + flags |= syscall.O_NOFOLLOW + } + return os.OpenFile(path, flags, 0) +} diff --git a/free/ci/internal/release/verify/open_unix_test.go b/free/ci/internal/release/verify/open_unix_test.go new file mode 100644 index 000000000..3df5648e8 --- /dev/null +++ b/free/ci/internal/release/verify/open_unix_test.go @@ -0,0 +1,60 @@ +//go:build unix + +package verify + +import ( + "os" + "path/filepath" + "syscall" + "testing" + "time" +) + +// The open refuses a leaf that is a symbolic link when it gets there, not only when an earlier Lstat +// looked (D-1198): this is the state a swap between the check and the open leaves behind. +func TestOpenCheckedRefusesASwappedLeaf(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "target") + if err := os.WriteFile(target, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + link := filepath.Join(dir, "link") + if err := os.Symlink(target, link); err != nil { + t.Skip("no symlinks here") + } + if f, err := openChecked(link, "link", false); err == nil { + _ = f.Close() + t.Fatal("a symbolic link leaf was opened") + } + f, err := openChecked(link, "link", true) // an operator trust file may be a link to a regular file + if err != nil { + t.Fatalf("a followed link to a regular file: %v", err) + } + _ = f.Close() +} + +// A FIFO that is there at open time is refused after a non-blocking open, with or without following. +func TestOpenCheckedRefusesAFifoAtOpenTime(t *testing.T) { + p := filepath.Join(t.TempDir(), "pipe") + if err := syscall.Mkfifo(p, 0o644); err != nil { + t.Skip("no fifo here") + } + for _, follow := range []bool{false, true} { + done := make(chan error, 1) + go func() { + f, err := openChecked(p, "pipe", follow) + if err == nil { + _ = f.Close() + } + done <- err + }() + select { + case err := <-done: + if err == nil { + t.Fatalf("follow=%v: a fifo was opened as a regular file", follow) + } + case <-time.After(5 * time.Second): + t.Fatalf("follow=%v: the open blocked on a fifo", follow) + } + } +} diff --git a/free/ci/internal/release/verify/trust.go b/free/ci/internal/release/verify/trust.go index c4330837a..a097a71eb 100644 --- a/free/ci/internal/release/verify/trust.go +++ b/free/ci/internal/release/verify/trust.go @@ -19,7 +19,6 @@ import ( "fmt" "io" "io/fs" - "os" "path/filepath" "regexp" "strconv" @@ -95,17 +94,10 @@ func operatorFile(path string) ([]byte, error) { if path == "" { return nil, nil } - st, err := os.Stat(path) + f, err := openChecked(path, path, true) // judged on the opened descriptor: a swap after a stat cannot block the read if errors.Is(err, fs.ErrNotExist) { return nil, nil } - if err == nil && !st.Mode().IsRegular() { - err = fmt.Errorf("%s is not a regular file", path) - } - if err != nil { - return nil, err - } - f, err := os.Open(path) if err != nil { return nil, err } diff --git a/free/ci/internal/roles/decide.go b/free/ci/internal/roles/decide.go index 29a800486..33169b2ca 100644 --- a/free/ci/internal/roles/decide.go +++ b/free/ci/internal/roles/decide.go @@ -32,6 +32,15 @@ func principalOf(label string) string { return strings.TrimPrefix(s, "cli:") } +// PrincipalOf is principalOf for callers outside the package: an audit label, with or without a trailing +// " role=..." note, reduced to the principal id. +func PrincipalOf(label string) string { + if f := strings.Fields(label); len(f) > 0 { + label = f[0] + } + return principalOf(label) +} + func (a Actor) decide(ctx context.Context, d decision) error { var err error if a.svc != nil && a.id != "" && a.kind != "" { diff --git a/free/ci/internal/roles/grants.go b/free/ci/internal/roles/grants.go index 867580a2a..8900b1a21 100644 --- a/free/ci/internal/roles/grants.go +++ b/free/ci/internal/roles/grants.go @@ -5,7 +5,8 @@ package roles // Inputs: an acting Actor (checked for roles.manage / token.manage) and the request. // Outputs: stored rows; the clear token text exactly once, from IssueToken. // Constraints: a token is 32 random bytes in base64url, stored only as its sha256; each mutation writes -// exactly one audit row in its own transaction, built by the pure check (no extra decision row); +// exactly one audit row in its own transaction, built by the pure check (no extra decision row); a +// refusal writes one ".refused" row with the reason key (signin.go, D-1276); // `--self` identities are issued only to Service.Self and record subject "self", all others // record the named principal; roles are the three named ones; a project list is explicit or "*". @@ -91,7 +92,7 @@ func (s *Service) mutation(by Actor, action, target, detail string) (store.Audit func (s *Service) Grant(ctx context.Context, by Actor, r Request) error { subject, err := s.check(ctx, by, r) if err != nil { - return err + return s.refused(ctx, by, "roles.grant", r.Principal, err) } in, err := s.mutation(by, "roles.grant", r.Principal, fmt.Sprintf("role=%s projects=%s subject=%s", r.Role, strings.Join(r.Projects, ","), subject)) if err != nil { @@ -104,7 +105,7 @@ func (s *Service) Grant(ctx context.Context, by Actor, r Request) error { // neither. func (s *Service) Revoke(ctx context.Context, by Actor, principal string) (bool, error) { if err := s.admin(ctx, by); err != nil { - return false, err + return false, s.refused(ctx, by, "roles.revoke", principal, err) } in, err := s.mutation(by, "roles.revoke", principal, "grant and live tokens revoked") if err != nil { @@ -137,14 +138,14 @@ func (s *Service) List(ctx context.Context, by Actor) ([]store.Grant, error) { func (s *Service) IssueToken(ctx context.Context, by Actor, r Request) (string, store.Token, error) { subject, err := s.check(ctx, by, r) if err != nil { - return "", store.Token{}, err + return "", store.Token{}, s.refused(ctx, by, "token.issue", r.Principal, err) } ttl := r.TTL if ttl == 0 { ttl = DefaultTokenTTL } if ttl < MinTokenTTL || ttl > MaxTokenTTL { - return "", store.Token{}, fail(ReasonBadRequest, "token lifetime must be between %s and %s", MinTokenTTL, MaxTokenTTL) + return "", store.Token{}, s.refused(ctx, by, "token.issue", r.Principal, fail(ReasonBadRequest, "token lifetime must be between %s and %s", MinTokenTTL, MaxTokenTTL)) } raw := make([]byte, 32) if _, err = rand.Read(raw); err != nil { @@ -168,7 +169,7 @@ func (s *Service) IssueToken(ctx context.Context, by Actor, r Request) (string, // RevokeToken ends a live token by id; false means no such live token. func (s *Service) RevokeToken(ctx context.Context, by Actor, id string) (bool, error) { if err := s.admin(ctx, by); err != nil { - return false, err + return false, s.refused(ctx, by, "token.revoke", id, err) } in, err := s.mutation(by, "token.revoke", id, "token revoked") if err != nil { diff --git a/free/ci/internal/roles/identity.go b/free/ci/internal/roles/identity.go index cc4cd5663..a31b90d1c 100644 --- a/free/ci/internal/roles/identity.go +++ b/free/ci/internal/roles/identity.go @@ -19,8 +19,23 @@ import ( "github.com/nself-org/plugins/free/ci/internal/store" ) -// ActorFromPeer maps a user certificate to its actor. +// ActorFromPeer maps a user certificate to its actor. A refusal writes a rate-limited sign-in failure +// row (signin.go). func (s *Service) ActorFromPeer(ctx context.Context, cert *x509.Certificate) (Actor, error) { + a, err := s.actorFromPeer(ctx, cert) + if err != nil { + who := "certificate:unknown" + if cert != nil { + if id, kind := pki.IdentityOf(cert); kind == pki.KindUser && id != "" { + who = "user:" + id + } + } + s.auditSignInFailure(ctx, who, "certificate", err) + } + return a, err +} + +func (s *Service) actorFromPeer(ctx context.Context, cert *x509.Certificate) (Actor, error) { if cert == nil { return Actor{}, fail(ReasonBadCert, "no client certificate") } @@ -53,13 +68,19 @@ func (s *Service) ActorFromPeer(ctx context.Context, cert *x509.Certificate) (Ac return Actor{id: id, kind: KindUser, role: Role(g.Role), projects: g.Projects, serial: serial, notAfter: cert.NotAfter, svc: s}, nil } -// ActorFromToken maps a scoped token to its actor. +// ActorFromToken maps a scoped token to its actor. A refusal writes a rate-limited sign-in failure row +// (signin.go), so a token guessing run shows in the audit log. func (s *Service) ActorFromToken(ctx context.Context, token string) (Actor, error) { t, ok, err := s.St.TokenByDigest(ctx, digestOf(token)) if err != nil { return Actor{}, err } if err = s.tokenState(t, ok); err != nil { + who := "token:unknown" + if ok { + who = "token:" + t.ID + ":" + t.Principal + } + s.auditSignInFailure(ctx, who, "token", err) return Actor{}, err } return Actor{id: t.Principal, kind: KindToken, role: Role(t.Role), projects: t.Projects, tokenID: t.ID, svc: s}, nil diff --git a/free/ci/internal/roles/roles.go b/free/ci/internal/roles/roles.go index 0bfc4952f..dad69565a 100644 --- a/free/ci/internal/roles/roles.go +++ b/free/ci/internal/roles/roles.go @@ -92,6 +92,8 @@ type Service struct { Log *audit.Log Now func() time.Time Self string // the operator's own principal id; `--self` identities are issued only to it + + lim signInLimiter // the rate limit of the sign-in failure rows (signin.go) } // New returns a Service over st writing audit rows through log. diff --git a/free/ci/internal/roles/signin.go b/free/ci/internal/roles/signin.go new file mode 100644 index 000000000..effce2569 --- /dev/null +++ b/free/ci/internal/roles/signin.go @@ -0,0 +1,134 @@ +package roles + +// Purpose: audit rows for what the decision path never sees (D-1276): a refused sign-in (a certificate or a +// token that does not resolve to an actor) and a refused admin mutation (a validation or role refusal). +// Inputs: who tried, how, and the E687 error; the peer address when the transport gave one (WithPeer). +// Outputs: a "signin.fail" row per bucket and window, and a ".refused" row per refused mutation. +// Constraints: a sign-in failure row is rate limited, so a guessing run cannot flood the log: one row per +// peer and kind per window, one more row per window of the whole log's failures once a global +// budget is spent; the rows that were held back are counted in the next row (suppressed=N). A +// row names the reason key, the peer and the principal or token id when one is known, never the +// token text. Writing is best effort: the sign-in was refused already, so a failed append does +// not change the answer. Rows for a store failure (no reason key) are not written. +// +// Audit rows of a remote mutation (D-1276, decision): a mutation that calls Authorize and then writes its +// own row leaves two rows on purpose. The authorize row is the decision (allow or deny, with the role and +// project asked for) and exists whether or not the mutation follows; the mutation row is written first and +// fails closed, so no action happens without it. Both carry the same actor, and the target of the +// mutation row names the object. One combined row would drop the denial of a request that never reaches +// its mutation. Member-level decisions write no authorize row, so those mutations leave one row. + +import ( + "context" + "fmt" + "strings" + "sync" + "time" + + "github.com/nself-org/plugins/free/ci/internal/audit" +) + +const ( + signInWindow = 30 * time.Second + signInBudget = 20 // rows per window across every peer + signInTrack = 4096 // peers remembered before the table is cleared +) + +type signInBucket struct { + start time.Time + suppressed int +} + +// signInLimiter is the table behind the rate limit; the zero value is ready. +type signInLimiter struct { + mu sync.Mutex + by map[string]*signInBucket + globalAt time.Time + globalUsed int + globalHeld int +} + +// allow reports whether a row may be written for key now, and how many rows were held back since the +// last one that was written for it. +func (l *signInLimiter) allow(key string, now time.Time) (bool, int) { + l.mu.Lock() + defer l.mu.Unlock() + if l.by == nil || len(l.by) >= signInTrack { + l.by = map[string]*signInBucket{} + } + b := l.by[key] + if b != nil && now.Sub(b.start) < signInWindow { + b.suppressed++ + return false, 0 + } + if now.Sub(l.globalAt) >= signInWindow { + l.globalAt, l.globalUsed = now, 0 + } + if l.globalUsed >= signInBudget { + if b != nil { + b.suppressed++ + } + l.globalHeld++ + return false, 0 + } + l.globalUsed++ + held := l.globalHeld + l.globalHeld = 0 + if b != nil { + held += b.suppressed + } + l.by[key] = &signInBucket{start: now} + return true, held +} + +type peerKey struct{} + +// WithPeer records the remote address of a request, so a sign-in failure row names it and the rate limit +// is per peer. +func WithPeer(ctx context.Context, addr string) context.Context { + return context.WithValue(ctx, peerKey{}, addr) +} + +// auditSignInFailure writes the rate-limited row of a refused sign-in. kind is "token" or "certificate". +func (s *Service) auditSignInFailure(ctx context.Context, who, kind string, err error) { + reason := ReasonOf(err) + if s.Log == nil || reason == "" { + return + } + peer, _ := ctx.Value(peerKey{}).(string) + now := time.Now() + if s.Now != nil { + now = s.Now() + } + ok, held := s.lim.allow(kind+"|"+peer, now) + if !ok { + return + } + detail := "reason=" + reason + if peer != "" { + detail += fmt.Sprintf(" peer=%q", peer) + } + if held > 0 { + detail += fmt.Sprintf(" suppressed=%d", held) + } + _ = s.Log.Append(ctx, audit.Event{Actor: who, Action: "signin.fail", Target: kind, Detail: detail}) +} + +// refused writes the row of a refused mutation and returns err unchanged. Only a refusal with a reason +// key is a row: a store failure is not a decision. +func (s *Service) refused(ctx context.Context, by Actor, action, target string, err error) error { + reason := ReasonOf(err) + if err == nil || s.Log == nil || reason == "" { + return err + } + who := "unknown" + if by.id != "" && by.kind != "" { + who = by.Label() + } + if r := []rune(target); len(r) > 64 { + target = string(r[:64]) + } + target = strings.ReplaceAll(fmt.Sprintf("%q", target), " ", "_") + _ = s.Log.Append(ctx, audit.Event{Actor: who, Action: action + ".refused", Target: target, Detail: "reason=" + reason}) + return err +} diff --git a/free/ci/internal/roles/signin_test.go b/free/ci/internal/roles/signin_test.go new file mode 100644 index 000000000..eb91fb44b --- /dev/null +++ b/free/ci/internal/roles/signin_test.go @@ -0,0 +1,164 @@ +package roles + +// Purpose: tests of the audit rows of refused sign-ins and refused admin mutations (D-1276), and of the +// documented two-row pattern of a mutation that authorizes first. + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/nself-org/plugins/free/ci/internal/audit" +) + +func auditEvent(a Actor, action, target string) audit.Event { + return audit.Event{Actor: a.Label(), Action: action, Target: target} +} + +func (r *rig) actions() []string { + var out []string + for _, e := range r.auditRows() { + out = append(out, e.Action) + } + return out +} + +func TestSignInFailuresWriteARateLimitedRow(t *testing.T) { + r := newRig(t) + ctx := WithPeer(context.Background(), "203.0.113.9") + secret := "guess-0123456789" + for i := 0; i < 50; i++ { // a token guessing run + if _, err := r.svc.ActorFromToken(ctx, fmt.Sprintf("%s-%d", secret, i)); ReasonOf(err) != ReasonTokenUnknown { + t.Fatalf("guess %d: %v", i, err) + } + } + rows := r.auditRows() + if len(rows) != 1 || rows[0].Action != "signin.fail" || rows[0].Actor != "token:unknown" || rows[0].Target != "token" { + t.Fatalf("a guessing run leaves one row per window: %+v", rows) + } + if d := rows[0].Detail; !strings.Contains(d, "reason=token_unknown") || !strings.Contains(d, `peer="203.0.113.9"`) || strings.Contains(d, secret) { + t.Fatalf("the row names the reason and peer, never the token: %q", d) + } + r.now = r.now.Add(signInWindow) + if _, err := r.svc.ActorFromToken(ctx, "guess-again"); err == nil { + t.Fatal("a guess was accepted") + } + rows = r.auditRows() + if len(rows) != 2 || !strings.Contains(rows[1].Detail, "suppressed=49") { + t.Fatalf("the next window's row counts the rows held back: %+v", rows) + } + if _, err := r.svc.ActorFromToken(WithPeer(context.Background(), "198.51.100.1"), "other-peer"); err == nil || len(r.auditRows()) != 3 { + t.Fatalf("another peer has its own budget: %d rows", len(r.auditRows())) + } +} + +func TestSignInFailureBudgetBoundsManyPeers(t *testing.T) { + r := newRig(t) + for i := 0; i < 5*signInBudget; i++ { + ctx := WithPeer(context.Background(), fmt.Sprintf("10.0.%d.%d", i/200, i%200)) + _, _ = r.svc.ActorFromToken(ctx, "nope") + } + if n := len(r.auditRows()); n != signInBudget { + t.Fatalf("%d rows, want the budget of %d", n, signInBudget) + } +} + +func TestSignInFailureRowsForKnownTokensAndCertificates(t *testing.T) { + r := newRig(t) + ctx := context.Background() + tok, issued, err := r.svc.IssueToken(ctx, r.op(), Request{Principal: "bob", Role: Member, Projects: []string{"p1"}}) + if err != nil { + t.Fatal(err) + } + r.now = r.now.Add(DefaultTokenTTL + time.Hour) + if _, err = r.svc.ActorFromToken(ctx, tok); ReasonOf(err) != ReasonTokenExpired { + t.Fatal(err) + } + cert, _ := userCert(t, r, "dave") + if _, err = r.svc.ActorFromPeer(ctx, cert); ReasonOf(err) != ReasonNoGrant { + t.Fatal(err) + } + r.now = r.now.Add(signInWindow) // the certificate bucket is spent for this window + if _, err = r.svc.ActorFromPeer(ctx, nil); ReasonOf(err) != ReasonBadCert { + t.Fatal(err) + } + rows := r.auditRows() + if len(rows) != 4 { // the token issue, then one row per failed kind + t.Fatalf("rows: %v", r.actions()) + } + want := []struct{ actor, target, reason string }{ + {"token:" + issued.ID + ":bob", "token", "token_expired"}, + {"user:dave", "certificate", "no_grant"}, + {"certificate:unknown", "certificate", "bad_certificate"}, + } + for i, w := range want { + e := rows[i+1] + if e.Action != "signin.fail" || e.Actor != w.actor || e.Target != w.target || !strings.Contains(e.Detail, "reason="+w.reason) { + t.Errorf("row %d: %+v, want %+v", i+1, e, w) + } + } +} + +func TestSuccessfulSignInWritesNoRow(t *testing.T) { + r := newRig(t) + a := r.tokenActor("m", Member, "p1") + before := len(r.auditRows()) + if _, err := r.svc.ActorFromToken(context.Background(), "x"); err == nil || a.ID() != "m" { + t.Fatal("setup") + } + if len(r.auditRows()) != before+1 { + t.Fatalf("only the failure writes a row: %v", r.actions()) + } +} + +func TestRefusedAdminMutationsWriteARow(t *testing.T) { + r := newRig(t) + ctx := context.Background() + member := r.tokenActor("m", Member, AllProjects) + before := len(r.auditRows()) + _ = r.svc.Grant(ctx, r.op(), Request{Principal: "Bad Name", Role: Member, Projects: []string{"p"}}) + _ = r.svc.Grant(ctx, member, Request{Principal: "x", Role: Admin, Projects: []string{AllProjects}}) + _, _, _ = r.svc.IssueToken(ctx, r.op(), Request{Principal: "bob", Role: Member, Projects: []string{"p"}, TTL: time.Second}) + _, _ = r.svc.Revoke(ctx, member, "bob") + _, _ = r.svc.RevokeToken(ctx, member, "tok-1") + rows := r.auditRows()[before:] + want := []struct{ actor, action, reason string }{ + {r.op().Label(), "roles.grant.refused", "bad_request"}, + {member.Label(), "roles.grant.refused", "role"}, + {r.op().Label(), "token.issue.refused", "bad_request"}, + {member.Label(), "roles.revoke.refused", "role"}, + {member.Label(), "token.revoke.refused", "role"}, + } + if len(rows) != len(want) { + t.Fatalf("rows: %+v", rows) + } + for i, w := range want { + if rows[i].Actor != w.actor || rows[i].Action != w.action || rows[i].Detail != "reason="+w.reason { + t.Errorf("row %d: %+v, want %+v", i, rows[i], w) + } + } + if rows[0].Target != `"Bad_Name"` { + t.Errorf("the refused principal is quoted: %q", rows[0].Target) + } +} + +// A release-level remote mutation leaves the authorize row and its own row, in that order (the documented +// pattern): the decision is recorded whether or not the mutation follows. +func TestAuthorizeThenMutationLeavesTwoRows(t *testing.T) { + r := newRig(t) + ctx := context.Background() + a := r.tokenActor("rel", Release, "p1") + before := len(r.auditRows()) + if err := Authorize(ctx, a, Release, "p1"); err != nil { + t.Fatal(err) + } + if err := r.svc.Log.Append(ctx, auditEvent(a, "run.submit", "p1")); err != nil { + t.Fatal(err) + } + rows := r.auditRows()[before:] + if len(rows) != 2 || rows[0].Action != "authorize" || rows[1].Action != "run.submit" || rows[0].Actor != rows[1].Actor { + t.Fatalf("two rows, one actor: %+v", rows) + } +} diff --git a/free/ci/internal/run/coordinator.go b/free/ci/internal/run/coordinator.go index df19d313b..c6d22b680 100644 --- a/free/ci/internal/run/coordinator.go +++ b/free/ci/internal/run/coordinator.go @@ -221,6 +221,7 @@ func (c *Coordinator) Run(ctx context.Context) (Result, error) { sess.Admission.Force = sess.Admission.Force || c.Opts.Force sess.Plan, sess.Record = c.Plan, c.Opts.Record sess.PolicyRun, sess.Interactive, sess.ProviderRepo = c.Opts.PolicyRun, c.Opts.Interactive, c.Opts.ProviderRepo + sess.User, sess.Team = c.Opts.User, c.Opts.Team sess.Placement, sess.Queue = pol.Placement, pol.Queue sess.Capacity = c.Capacity if sess.Capacity == nil { diff --git a/free/ci/internal/run/dispatch_identity_test.go b/free/ci/internal/run/dispatch_identity_test.go new file mode 100644 index 000000000..c50d98b04 --- /dev/null +++ b/free/ci/internal/run/dispatch_identity_test.go @@ -0,0 +1,24 @@ +package run + +// Purpose: the scheduler job carries the run's user and team (D-1044), so the user and team limits +// count the attempts of that user and team, not every hook attempt as one bucket. + +import ( + "testing" + + "github.com/nself-org/plugins/free/ci/internal/model" + "github.com/nself-org/plugins/free/ci/internal/store" +) + +func TestSchedJobCarriesUserAndTeam(t *testing.T) { + job := model.Job{Kind: "test", Run: []string{"true"}} + rd := &runDispatch{sess: &Session{RunID: "run1", User: "mia", Team: "acme"}, row: store.PipelineRow{ID: "run1", Project: "acme/web", SourceTrust: "owner"}} + got := rd.schedJob(AttemptSpec{Job: &job, N: 1, Foreground: true}, "run1.a.a1", "run1.a", 1, 256) + if got.User != "mia" || got.Team != "acme" { + t.Fatalf("job identity = %q, %q; want mia, acme", got.User, got.Team) + } + rd.sess = &Session{RunID: "run1"} + if got = rd.schedJob(AttemptSpec{Job: &job, N: 1, Foreground: true}, "run1.a.a1", "run1.a", 1, 256); got.User != "" || got.Team != "" { + t.Fatalf("an unnamed run must keep the shared bucket, got %q, %q", got.User, got.Team) + } +} diff --git a/free/ci/internal/run/dispatch_world.go b/free/ci/internal/run/dispatch_world.go index d0d2e55cb..1522e16ab 100644 --- a/free/ci/internal/run/dispatch_world.go +++ b/free/ci/internal/run/dispatch_world.go @@ -59,6 +59,7 @@ func (rd *runDispatch) schedJob(s AttemptSpec, id, row string, cpu, mem int) sch if rd.sess.AdmitTrust != "" { // the row records the requested class; placement uses the class the run was admitted at j.Trust = rd.sess.AdmitTrust } + j.User, j.Team = rd.sess.User, rd.sess.Team j.Pipeline.Band = attemptBand(s.Band, rd.row.Trigger) j.Accelerators = nil j.Pipeline.Record = rd.sess.Record diff --git a/free/ci/internal/run/options.go b/free/ci/internal/run/options.go index 76240301c..b0ca00d14 100644 --- a/free/ci/internal/run/options.go +++ b/free/ci/internal/run/options.go @@ -55,6 +55,10 @@ type Options struct { // PolicyRun carries per-run policy values into PolicyRequest.Run (filled by P7-AUTO-29 from // `--where`, `--prefer` and `--runner`); nil means none. PolicyRun RunValues + // User and Team are who the run's jobs count against in the scheduler's user and team limits + // (D-1044). The coordinator fills them from the submitter of an intake row; empty means the run + // has no named identity (a webhook or a bare local run) and counts in the shared bucket. + User, Team string } // RunOptions is the name the cmd layer and its option hooks use. diff --git a/free/ci/internal/run/pseudonym_hook_test.go b/free/ci/internal/run/pseudonym_hook_test.go new file mode 100644 index 000000000..2209d306a --- /dev/null +++ b/free/ci/internal/run/pseudonym_hook_test.go @@ -0,0 +1,49 @@ +package run + +// Purpose: an attempt start registers the operator pseudonym key of its CI home (D-1198), so the +// builder ids and status text published after it are keyed. + +import ( + "bytes" + "context" + "os" + "path/filepath" + "testing" + + "github.com/nself-org/plugins/free/ci/internal/attest" +) + +func TestAttemptStartRegistersThePseudonymKey(t *testing.T) { + t.Cleanup(func() { _ = attest.UsePseudonymKey(nil); pseudonymHome.Store(nil) }) + pseudonymHome.Store(nil) + home := t.TempDir() + if err := (attestStoreHook{}).AttemptStart(context.Background(), AttemptInfo{Home: home}); err != nil { + t.Fatal(err) + } + key, err := os.ReadFile(filepath.Join(home, attest.PseudonymKeyFile)) + if err != nil || len(key) != 32 { + t.Fatalf("key file: %d bytes, %v", len(key), err) + } + facts := attest.RepoFacts{Visibility: attest.Public, RepoID: "local:abc"} + _ = attest.UsePseudonymKey(nil) + unkeyed := attest.ProfileFor(facts).RunnerID("host-1") + _ = attest.UsePseudonymKey(key) + want := attest.ProfileFor(facts).RunnerID("host-1") + _ = attest.UsePseudonymKey(nil) + pseudonymHome.Store(nil) + if err = (attestStoreHook{}).AttemptStart(context.Background(), AttemptInfo{Home: home}); err != nil { + t.Fatal(err) + } + if got := attest.ProfileFor(facts).RunnerID("host-1"); got != want || got == unkeyed || !bytes.HasPrefix([]byte(got), []byte("runner-")) { + t.Fatalf("profile after attempt start: %q (keyed %q, unkeyed %q)", got, want, unkeyed) + } + // a key file of the wrong shape fails the attempt rather than publishing unkeyed + bad := t.TempDir() + if err = os.WriteFile(filepath.Join(bad, attest.PseudonymKeyFile), []byte("short"), 0o600); err != nil { + t.Fatal(err) + } + pseudonymHome.Store(nil) + if err = (attestStoreHook{}).AttemptStart(context.Background(), AttemptInfo{Home: bad}); err == nil { + t.Fatal("a bad key file was accepted") + } +} diff --git a/free/ci/internal/run/remote_await.go b/free/ci/internal/run/remote_await.go index 52e493325..f0cb9046c 100644 --- a/free/ci/internal/run/remote_await.go +++ b/free/ci/internal/run/remote_await.go @@ -12,8 +12,8 @@ package run // end without one (lost) or the context. A cancel moves the attempt to cancelled // first (the lease row is gone, so a late result is fenced), then stops the // carrier and waits for it, so the node's job process group is killed before -// the run exits. An infra failure reported by a node is not retried (it is -// finalizing, which has no path back to the queue); a lost session is. +// the run exits. An infra failure or an unasked cancel reported by a node ends the attempt lost +// (remote_infra.go), like a lost session, so the CI-40 retry applies to both. import ( "context" @@ -258,19 +258,7 @@ func (rs *runState) remoteResult(ctx context.Context, pj plan.PlannedJob, id str } return rs.stopped(o, pj.ID) } - class := model.FailureClass("code") - exit := 1 - if ev.Result == "error" { - class = "infra" - } - for _, c := range ev.Checks { - if c.FailureClass != "" { - class = c.FailureClass - } - if c.Exit != nil { - exit = *c.Exit - } - } + class, exit := evidenceFailure(ev) o.ExitCode, o.FailureClass = exit, class d := diagnose.Classify(diagnose.Input{ExecClass: class, ExitCode: exit, Tail: o.Tail}) o.Diagnosis, o.Remediation = d.Signature, d.Remediation diff --git a/free/ci/internal/run/remote_await_test.go b/free/ci/internal/run/remote_await_test.go index 47206b6f7..39df9ea80 100644 --- a/free/ci/internal/run/remote_await_test.go +++ b/free/ci/internal/run/remote_await_test.go @@ -169,8 +169,14 @@ func TestRemoteAwaitUnreadableEvidenceIsAnErrorNeverAPass(t *testing.T) { if err := e.sink.Result(context.Background(), protocol.Result{LeaseID: e.lease, Epoch: uint64(e.epoch), Evidence: raw}); err != nil { t.Fatalf("%s: %v", name, err) } - if ev := <-ra.result; ev.Result != "error" { - t.Errorf("%s: result %q, want error", name, ev.Result) + // an unreadable document is an error result, which is an infra failure: the attempt is lost (D-1103) and never a pass + select { + case ev := <-ra.result: + t.Errorf("%s: result %q delivered, want the attempt lost", name, ev.Result) + default: + if got := e.state(t); got != "lost" { + t.Errorf("%s: state %s, want lost", name, got) + } } } } diff --git a/free/ci/internal/run/remote_infra.go b/free/ci/internal/run/remote_infra.go new file mode 100644 index 000000000..cdd11578f --- /dev/null +++ b/free/ci/internal/run/remote_infra.go @@ -0,0 +1,115 @@ +package run + +// Purpose: what the remote sink does with a result that asks for a re-run instead of a verdict +// (D-1103): an infra failure the node reports, or a cancel nobody asked for (a drain, an agent +// self-cancel). Such an attempt is lost, not finalizing, so the CI-40 infra retry can queue it +// again; finalizing has no path back to the queue. +// Inputs: the node's checked evidence and its protocol result. +// Outputs: the attempt moved to lost, and the run told through the carrier-lost channel. +// Constraints: the lease epoch is fenced first, as for any result. A result frame that carries a +// signature is verified against the node registry (attest.VerifyFrame); one the key does not verify is +// an infra failure of the node, never a verdict. An unsigned frame is accepted as before: the agent +// contract (P7-TRUST-20) lets a node without the signing-eligible marker complete unsigned, and the +// pinned ssh channel authenticates it. The release gate still refuses unsigned remote evidence. + +import ( + "context" + "errors" + + "github.com/nself-org/plugins/free/ci/internal/attest" + "github.com/nself-org/plugins/free/ci/internal/model" + "github.com/nself-org/plugins/free/ci/internal/protocol" + "github.com/nself-org/plugins/free/ci/internal/store" +) + +// nodeReported is the cause of a lost attempt whose node said so itself. A reported infra +// failure counts against the node's breaker; a cancel the node chose does not. +type nodeReported struct { + msg string + infra bool +} + +func (e *nodeReported) Error() string { return e.msg } + +var ( + errNodeInfra = &nodeReported{msg: "the node reported an infrastructure failure", infra: true} + errNodeCancelled = &nodeReported{msg: "the node cancelled the job without being asked"} +) + +// signatureCause is why a signed frame must not be trusted, or nil for an unsigned frame or one the +// registry key verifies. +func (s *remoteSink) signatureCause(r protocol.Result) error { + if r.Signature.KeyID == "" && r.Signature.Value == "" { + return nil + } + led := func() attest.Ledger { return store.AttestLedger{S: s.st} } + if err := attest.VerifyFrame(led, r.Evidence, r.Signature.KeyID, r.Signature.Value); err != nil { + return &nodeReported{msg: "the node's result signature was refused: " + string(attest.RemoteReasonOf(err)), infra: true} + } + return nil +} + +// evidenceFailure is the failure class and exit code a non-pass evidence document settles to: an +// error result is infra and a check's own class overrides, the last check that names one winning. +func evidenceFailure(ev model.Evidence) (class model.FailureClass, exit int) { + class, exit = "code", 1 + if ev.Result == "error" { + class = "infra" + } + for _, c := range ev.Checks { + if c.FailureClass != "" { + class = c.FailureClass + } + if c.Exit != nil { + exit = *c.Exit + } + } + return class, exit +} + +// unsettledCause is why the attempt must be re-run instead of settled from ev, or nil. A cancel the +// run asked for is the run's to settle. +func (ra *remoteAttempt) unsettledCause(ev model.Evidence) error { + switch ev.Result { + case "pass": + return nil + case "cancelled": + ra.mu.Lock() + asked := ra.cancelled + ra.mu.Unlock() + if asked { + return nil + } + return errNodeCancelled + } + if class, _ := evidenceFailure(ev); class == "infra" { + return errNodeInfra + } + return nil +} + +// failLease fails the lease of a fenced result: the attempt becomes lost (as when a carrier ends) +// and the run is told, so remoteLost parks it for the retry. +func (s *remoteSink) failLease(ctx context.Context, ra *remoteAttempt, r protocol.Result, cause error) error { + if err := s.eng.Fence(ctx, r.LeaseID, int64(r.Epoch)); err != nil { + return err + } + state, epoch, err := s.st.AttemptStateEpoch(ctx, ra.attemptID) + if err != nil { + return err + } + if state != "running" { + return fail(CodeRun, "result for lease %s while the attempt is %s", r.LeaseID, state) + } + if err = s.st.Transition(ctx, ra.attemptID, state, "lost", store.TransitionDetail{Epoch: epoch, FailureClass: "infra", Reason: cause.Error()}); err != nil { + return err + } + s.ended(ra.attemptID, cause) + return nil +} + +// reportedByNode says whether err is a loss the node reported itself. +func reportedByNode(err error) (*nodeReported, bool) { + var n *nodeReported + return n, errors.As(err, &n) +} diff --git a/free/ci/internal/run/remote_infra_test.go b/free/ci/internal/run/remote_infra_test.go new file mode 100644 index 000000000..5204b21bb --- /dev/null +++ b/free/ci/internal/run/remote_infra_test.go @@ -0,0 +1,149 @@ +package run + +// Purpose: D-1103 tests: a result a node reports as an infra failure, or a cancel the operator did +// not ask for, ends the attempt lost (the CI-40 retry path) and never finalizing, which has no way +// back to the queue. +// Inputs: a private store, a leased remote attempt, the remote sink. +// Outputs: assertions on the attempt state and the run's rendezvous channels. + +import ( + "context" + "encoding/json" + "testing" + + "github.com/nself-org/plugins/free/ci/internal/model" + "github.com/nself-org/plugins/free/ci/internal/protocol" +) + +func evWith(result string, checks ...model.Check) json.RawMessage { + raw, _ := json.Marshal(model.Evidence{Schema: "ci.evidence/v1", Revision: testRev, Attempt: &model.AttemptRef{ID: "run1.jobA.a1", N: 1}, Result: model.Result(result), Checks: checks}) + return raw +} + +// lostCause is the cause the run was told, failing at once when it was told nothing. +func lostCause(t *testing.T, ra *remoteAttempt) error { + t.Helper() + select { + case cause := <-ra.lost: + return cause + default: + t.Fatal("the run was not told the carrier is lost") + return nil + } +} + +func TestRemoteInfraResultTakesTheLostPath(t *testing.T) { + cases := map[string]struct { + raw json.RawMessage + cancelled bool // the run asked the carrier to stop + lost bool + }{ + "error result": {raw: evWith("error"), lost: true}, + "fail with an infra check": {raw: evWith("fail", model.Check{ID: "c", JobID: "j", Result: "fail", FailureClass: "infra"}), lost: true}, + "error with a code check": {raw: evWith("error", model.Check{ID: "c", JobID: "j", Result: "error", FailureClass: "code"}), lost: false}, + "fail with a code check": {raw: evWith("fail", model.Check{ID: "c", JobID: "j", Result: "fail", FailureClass: "code"}), lost: false}, + "unreadable evidence": {raw: json.RawMessage(`"x"`), lost: true}, + "cancel nobody asked for": {raw: evWith("cancelled"), lost: true}, + "cancel the run asked for": {raw: evWith("cancelled"), cancelled: true, lost: false}, + "pass": {raw: evWith("pass"), lost: false}, + } + for name, c := range cases { + e := newRemoteEnv(t) + ra := e.sink.expect(e.id, testRev) + if c.cancelled { + e.sink.cancelCarrier(e.id) + } + if err := e.sink.Result(context.Background(), protocol.Result{LeaseID: e.lease, Epoch: uint64(e.epoch), Evidence: c.raw}); err != nil { + t.Fatalf("%s: %v", name, err) + } + want := "finalizing" + if c.lost { + want = "lost" + } + if got := e.state(t); got != want { + t.Errorf("%s: state %s, want %s", name, got, want) + } + select { + case cause := <-ra.lost: + if !c.lost { + t.Errorf("%s: reported lost (%v) but the result is the run's to settle", name, cause) + } + default: + if c.lost { + t.Errorf("%s: the run was not told the carrier is lost", name) + } + } + if c.lost { + select { + case ev := <-ra.result: + t.Errorf("%s: a lost attempt also delivered a result %q", name, ev.Result) + default: + } + } + } +} + +func TestRemoteInfraResultIsFencedAndSingle(t *testing.T) { + e := newRemoteEnv(t) + e.sink.expect(e.id, testRev) + ctx := context.Background() + if err := e.sink.Result(ctx, protocol.Result{LeaseID: e.lease, Epoch: uint64(e.epoch) + 1, Evidence: evWith("error")}); err == nil { + t.Fatal("an infra result for another epoch was accepted") + } + if got := e.state(t); got == "lost" { + t.Fatalf("a stale infra result moved the attempt to %s", got) + } + if err := e.sink.Result(ctx, protocol.Result{LeaseID: e.lease, Epoch: uint64(e.epoch), Evidence: evWith("error")}); err != nil { + t.Fatal(err) + } + if err := e.sink.Result(ctx, protocol.Result{LeaseID: e.lease, Epoch: uint64(e.epoch), Evidence: evWith("error")}); err == nil { + t.Fatal("a second infra result for the same lease was accepted") + } +} + +// A node-reported infra failure counts against the node; a cancel it chose itself does not. +func TestRemoteInfraResultCountsAgainstTheNode(t *testing.T) { + e := newRemoteEnv(t) + ra := e.sink.expect(e.id, testRev) + if err := e.sink.Result(context.Background(), protocol.Result{LeaseID: e.lease, Epoch: uint64(e.epoch), Evidence: evWith("error")}); err != nil { + t.Fatal(err) + } + if cause := lostCause(t, ra); !nodeFault(cause) { + t.Fatalf("cause %v is not a node fault", cause) + } + e2 := newRemoteEnv(t) + ra2 := e2.sink.expect(e2.id, testRev) + if err := e2.sink.Result(context.Background(), protocol.Result{LeaseID: e2.lease, Epoch: uint64(e2.epoch), Evidence: evWith("cancelled")}); err != nil { + t.Fatal(err) + } + if cause := lostCause(t, ra2); nodeFault(cause) { + t.Fatalf("a node-initiated cancel %v must not trip the node's breaker", cause) + } +} + +// A frame that carries a signature the node registry does not verify is an infra failure of the node, +// even when its document says pass; an unsigned frame keeps the old acceptance (P7-TRUST-20). +func TestRemoteResultSignatureMustVerifyWhenPresent(t *testing.T) { + for name, c := range map[string]struct { + sig protocol.Signature + want string + }{ + "unsigned": {want: "finalizing"}, + "signed by an unknown key": {sig: protocol.Signature{KeyID: "ci-node-0123456789abcdef", Value: "AAAA"}, want: "lost"}, + "key id without a value": {sig: protocol.Signature{KeyID: "ci-node-0123456789abcdef"}, want: "lost"}, + } { + e := newRemoteEnv(t) + ra := e.sink.expect(e.id, testRev) + if err := e.sink.Result(context.Background(), protocol.Result{LeaseID: e.lease, Epoch: uint64(e.epoch), Evidence: evWith("pass"), Signature: c.sig}); err != nil { + t.Fatalf("%s: %v", name, err) + } + if got := e.state(t); got != c.want { + t.Errorf("%s: state %s, want %s", name, got, c.want) + } + if c.want == "lost" { + if cause := lostCause(t, ra); !nodeFault(cause) { + t.Errorf("%s: cause %v must count against the node", name, cause) + } + } + } +} diff --git a/free/ci/internal/run/remote_settle.go b/free/ci/internal/run/remote_settle.go index a6f4af214..7891421bf 100644 --- a/free/ci/internal/run/remote_settle.go +++ b/free/ci/internal/run/remote_settle.go @@ -24,6 +24,9 @@ var nodeFaults = map[string]bool{"transport.ssh_session_lost": true, "transport. // nodeFault says whether a carrier error is the node's doing. The reason is the second word of an ssh // transport error ("E656 transport.ssh_session_lost: ..."), a position the message text cannot reach. func nodeFault(err error) bool { + if n, ok := reportedByNode(err); ok { + return n.infra + } f := strings.Fields(err.Error()) return len(f) >= 2 && strings.HasPrefix(f[0], "E65") && nodeFaults[strings.TrimSuffix(f[1], ":")] } diff --git a/free/ci/internal/run/remote_sink.go b/free/ci/internal/run/remote_sink.go index 0bcbfe8bd..918bae4fc 100644 --- a/free/ci/internal/run/remote_sink.go +++ b/free/ci/internal/run/remote_sink.go @@ -255,10 +255,17 @@ func (s *remoteSink) Result(ctx context.Context, r protocol.Result) error { if err := s.markRunning(ctx, ra); err != nil { return err } + ev := ra.checkEvidence(r.Evidence) + cause := s.signatureCause(r) + if cause == nil { + cause = ra.unsettledCause(ev) + } + if cause != nil { + return s.failLease(ctx, ra, r, cause) + } if err := s.eng.Result(ctx, r.LeaseID, int64(r.Epoch)); err != nil { return err } - ev := ra.checkEvidence(r.Evidence) select { case ra.result <- ev: default: diff --git a/free/ci/internal/run/session.go b/free/ci/internal/run/session.go index a09daedff..462fee0c1 100644 --- a/free/ci/internal/run/session.go +++ b/free/ci/internal/run/session.go @@ -48,7 +48,9 @@ type Session struct { // requested class but is placed as the operator-owned job it is. AdmitTrust model.TrustClass // Clock is the time the placement world is built at (nil = time.Now); tests inject it so a plan and a run can be compared. - Clock func() time.Time + Clock func() time.Time + // User and Team are Options.User and Options.Team: the dispatch hook sets them on every scheduler job. + User, Team string Interactive bool // a person waits at a terminal: only then does the implicit foreground queue cap apply // ProviderRepo is Options.ProviderRepo: the one repository whose hosted runners the world keeps ("" = none). ProviderRepo string diff --git a/free/ci/internal/run/signing_hook.go b/free/ci/internal/run/signing_hook.go index c775a29f7..d5c190d0b 100644 --- a/free/ci/internal/run/signing_hook.go +++ b/free/ci/internal/run/signing_hook.go @@ -7,7 +7,8 @@ package run // audits each promoted release and re-posts the affected statuses as errors. // Inputs: the run's store (taken from the first attempt that starts, or set with UseAttestStore); for // RevokeNode a store, an audit log and a node id. -// Outputs: the registered verifier; attest.Affected from RevokeNode. +// Outputs: the registered verifier; attest.Affected from RevokeNode; the operator pseudonym key the +// publication profile uses (attest.LoadPseudonymKey under the run's CI home, D-1198). // Constraints: with no store known the verifier refuses everything (fail closed); the store is only read // and written through store.AttestLedger. @@ -38,13 +39,35 @@ func init() { RegisterAttemptStart(attestStoreHook{}) } -// attestStoreHook points the verifier at the store of the attempt that is starting. +// attestStoreHook points the verifier at the store of the attempt that is starting, and the publication +// profile at the operator's pseudonym key under the run's CI home (D-1198). type attestStoreHook struct{} +var pseudonymHome atomic.Pointer[string] // the home whose key is registered + func (attestStoreHook) AttemptStart(_ context.Context, a AttemptInfo) error { if a.Store != nil { UseAttestStore(a.Store) } + return usePseudonymKey(a.Home) +} + +// usePseudonymKey registers the key of the CI home once per home. A home whose key cannot be created or +// read fails the attempt: publishing with the unkeyed pseudonym would let a host name dictionary +// recover the builder id. +func usePseudonymKey(home string) error { + dir := ciHome(home) + if cur := pseudonymHome.Load(); cur != nil && *cur == dir { + return nil + } + key, err := attest.LoadPseudonymKey(dir) + if err == nil { + err = attest.UsePseudonymKey(key) + } + if err != nil { + return fail(CodeRun, "the publication pseudonym key is not usable: %v", err) + } + pseudonymHome.Store(&dir) return nil }