Fix/verify signatures

.github/workflows/ci.yml

@@ -49,6 +49,15 @@ jobs:
           sudo apt-get update -qq
           sudo apt-get install -y -qq qemu-user-static binfmt-support
           test -e /proc/sys/fs/binfmt_misc/qemu-aarch64
+      # Client verification skips where the machine cannot run the container it
+      # needs, so that a developer on a runtime without a CPU cgroup sees what
+      # was checked rather than a daemon error. A skip here would be
+      # indistinguishable from a pass, and every client test would silently stop
+      # running if a runner image lost the capability. This makes their absence
+      # a failure, for the same reason binfmt_misc is asserted above.
+      - name: Require real client verification
+        if: runner.os == 'Linux'
+        run: echo "SNAILMAIL_REQUIRE_CLIENT_VERIFICATION=1" >> "$GITHUB_ENV"
       - run: make fmt
       - run: make vet
       - run: make lint

PLAN.md

@@ -829,6 +829,13 @@ Private material sits behind `KeyRef` with pluggable backends — Actions secret
 `pass`/1Password, age/SOPS in-repo, KMS, hardware token. Where the backend can
 sign remotely, the engine never holds key material.
 
+Two exist. `file` owns a key: it generates one and is the only thing that can
+produce it again, which is what a workstation wants. `env` owns nothing and
+resolves a key the environment supplies, which is what a CI runner needs, since
+the runner is discarded after every job and the key comes from a store that
+already holds it. Neither signs remotely, so the engine still holds material
+briefly in both; the backends that would not are the ones left.
+
 ## 9. Operations
 
 The verbs, and which part of the model each one touches:
@@ -1394,8 +1401,7 @@ snailmail keys new|publish|audit|rotate
 snailmail approval-key | blob-store | render | serve
 ```
 
-Planned: `import <url>` (adopt an existing repository into a manifest), `ui`,
-`server`. Every command that reports a result accepts `--json`.
+Planned: `server`. `import <url>` and `ui` are implemented. Every command that reports a result accepts `--json`.
 
 ## 14. Phases
 
@@ -1442,18 +1448,64 @@ rotation, plan-resolved deterministic signatures verified through apt
 Tier 1 is complete: rpm and apk landed, and every format the knowledge bundle
 marks signable is now signed — Debian's `InRelease` and `Release.gpg`, a yum
 `repomd.xml.asc`, an Alpine index per architecture, and a Helm `.prov` per
-chart, each verified by running the real client. Signing lives behind a
-`formats.Signer` interface rather than a switch in the engine, so a format
-either satisfies it or does not.
+chart, each verified by running the real client with the signature check on.
+Signing lives behind a `formats.Signer` interface rather than a switch in the
+engine, so a format either satisfies it or does not.
+
+That last clause was only true of Debian for a while. rpm, apk and Helm each
+signed correctly and were then verified by a client told not to check: dnf ran
+with `repo_gpgcheck=0`, apk with `--allow-untrusted`, and `helm pull` never read
+the `.prov` beside the chart. So a signature no client would accept verified
+green, which is the one thing repository signing exists to prevent. The
+verifiers now install the way the generated install instructions have always
+told a consumer to, and `internal/app/signed_verification_test.go` holds every
+signable format to it, because the hole was not that any one script was wrong
+but that nothing tied a format's signing to its verification.
 
 The format-and-host coupling that this phase owed is closed for its first
 cases: GitHub Pages serves signed Debian, rpm, apk, raw and Helm alongside PyPI.
 The matrix itself is declared in `host/support.go` rather than inferred, so the
 gaps that remain are readable rather than discovered — and each undeclared pair
-records why. Remaining: additional key backends and the TUI. The next formats
-are nix cache, cargo, go, and maven, in that order. `import` is implemented, and
-so is S3 beyond PyPI — see the paragraph below, which was written when it landed
-and outlived this sentence.
+records why. The next formats are nix cache, cargo, go, and maven, in that
+order.
+
+Both things this plan calls a TUI are built, and they are different things.
+§13's is the products-by-destinations matrix: `status --matrix` renders it once
+and `ui` draws it full screen, from one model over engine.StatusWorkspace, so
+the two cannot come to disagree about where a product is published. It is a
+viewer and not a console — everything that changes a workspace is a reviewed
+diff and a plan, and a keystroke is neither. Raw terminal handling is termios
+through golang.org/x/sys, already a dependency, so a full-screen view costs no
+new one.
+
+§7's is the setup wizard, built as it requires: it produces an argument list and nothing else, so what configures a repository is the same command
+either way, and it prints that command. Making it a flag-filler needed setup's
+flags declared once and read twice -- by the command that acts on them and the
+wizard that asks for them -- which is what stops the parallel config writer §7
+warns about. It is line-based rather than a full-screen interface, which needs
+no dependency and lets a scripted session be a test.
+
+Doing it surfaced a flag-grouping bug: --output was offered for every host, but
+an object store and Pages both refuse a path outright, so on those two it was a
+flag that could only make the repository invalid. Its own help had always named
+local and rsync. `import` is implemented, and so is S3 beyond PyPI — see
+the paragraph below, which was written when it landed and outlived this
+sentence.
+
+Key backends are no longer a single item. `KeyRef.Backend` was carried through
+every layer and read by nothing: `wire.NewSignerStore` returned the file store
+itself, and both the adapter and the manifest validator hard-rejected any other
+value. Selecting a backend is the composition root's job, so that is where the
+dispatch now lives, and the field decides something.
+
+The second backend is `env`, which resolves a key the environment supplies
+rather than one it owns — the CI case §8 names first among the backends private
+material sits behind. It generates nothing and deletes nothing, and says so when
+asked: a backend that reported success for either would let a rotation believe
+it had retired a key that every job still receives. What remains of §8's list is
+the backends that sign without releasing the key at all — KMS and a hardware
+token — which are a different shape of work, because the engine would assemble
+the OpenPGP packet around a signature made elsewhere.
 
 S3 beyond PyPI turned out not to be a declaration. The adapter no longer
 hardcodes `simple/index.html`: `host.Repository` carries `CommitPaths`, filled

README.md

@@ -38,6 +38,16 @@ go run ./cmd/snailmail apply
 `plan` writes what it intends to do; `apply` does it and records a publication
 ledger entry. Nothing reaches a host until a plan has been applied.
 
+`status --matrix` reports products against destinations — which package is
+published where, and at what version — and `snailmail ui` draws the same matrix
+full screen, refreshing on `r`. Both read what `status --json` emits, so neither
+can show something the JSON does not say.
+
+`setup <format>` with nothing else asks for what it needs, offering only the
+flags the chosen host uses, and prints the command it ran — which is the one to
+keep for CI. It fills flags and nothing more, so there is no second way to
+configure a repository. Add `--interactive` to ask when input is not a terminal.
+
 Each command prints the one that usually comes next, so the sequence can be
 followed without this page. The `git commit` in the middle is not incidental:
 desired state is reviewed as a diff, and `plan` reads committed state only.

adapters/signer/env/env.go

@@ -0,0 +1,207 @@
+// Package envsigner reads a signing key that was generated somewhere else and
+// handed to this process by its environment.
+//
+// The file backend owns a key: it generates one, writes it under the user's
+// data directory, and is the only thing that can produce it again. That is the
+// right shape for a workstation and the wrong one for CI, where the runner is
+// discarded after every job and the key has to arrive from a secret store that
+// already holds it. PLAN.md §8 names that case first among the backends private
+// material sits behind.
+//
+// So this backend generates nothing. It resolves a key that already exists, in
+// the same encrypted armored form the file backend writes, supplied either
+// directly or as a path to a mounted file.
+package envsigner
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"regexp"
+	"strings"
+	"time"
+
+	"github.com/shellcell/snailmail/signer"
+	apkrsa "github.com/shellcell/snailmail/signer/apkrsa"
+	openpgpsigner "github.com/shellcell/snailmail/signer/openpgp"
+)
+
+// Backend is the name a key reference carries to be resolved here.
+const Backend = "env"
+
+// maxMaterialBytes bounds what is read, so a variable or a path pointing at
+// something enormous fails cleanly rather than being pulled into memory. An
+// RSA4096 private key in the armored form the file backend writes is a few
+// kilobytes.
+const maxMaterialBytes = 1 << 20
+
+// referencePattern is the reference shape both backends share: the workspace
+// that owns the key, then the key's own name.
+//
+// Only the name is used here. The workspace half is validated and otherwise
+// ignored, because an environment variable named after a 64-character digest is
+// not something anyone would configure. That means one runner serving two
+// workspaces with a same-named key would hand both the same material -- and what
+// catches it is the fingerprint, not the reference: planning a signed repository
+// resolves the key and refuses when "private signer identity differs from
+// committed public key".
+var referencePattern = regexp.MustCompile(`^([a-f0-9]{64})/([a-z][a-z0-9-]*)$`)
+
+type Store struct {
+	lookup     func(string) (string, bool)
+	readFile   func(string) ([]byte, error)
+	passphrase func() ([]byte, error)
+}
+
+// New reads keys from the process environment.
+func New(passphrase func() ([]byte, error)) (*Store, error) {
+	if passphrase == nil {
+		return nil, errors.New("a passphrase provider is required")
+	}
+	return &Store{lookup: os.LookupEnv, readFile: os.ReadFile, passphrase: passphrase}, nil
+}
+
+// Variable is the environment variable a key's material is read from, and
+// Variable+"_FILE" is the path form.
+//
+// It is derived from the key's name rather than its reference because the
+// reference begins with a workspace digest, which nothing would want to type
+// into a CI configuration. Key names cannot contain an underscore, so mapping a
+// hyphen onto one cannot make two names collide.
+//
+// A runner that serves more than one workspace therefore has to keep their
+// variables apart itself, by running each job with only the keys it needs.
+func Variable(name string) string {
+	return "SNAILMAIL_KEY_" + strings.ToUpper(strings.ReplaceAll(name, "-", "_"))
+}
+
+// Generate refuses. A key this backend can see is one that already existed.
+func (store *Store) Generate(context.Context, signer.Ref, signer.Algorithm, string, time.Time, time.Duration) (signer.Generated, error) {
+	return signer.Generated{}, errors.New(
+		"the env backend cannot generate a signing key: create one with the file backend, or with whatever already holds it, and supply it through the environment")
+}
+
+// Delete refuses for the same reason. Removing the variable is the environment's
+// business, and reporting success without doing anything would let a rotation
+// believe it had retired a key that is still being handed to every job.
+func (store *Store) Delete(_ context.Context, ref signer.Ref) error {
+	name, err := keyName(ref)
+	if err != nil {
+		return err
+	}
+	return fmt.Errorf("the env backend cannot delete a signing key: unset %s where it is configured", Variable(name))
+}
+
+func (store *Store) Public(ctx context.Context, ref signer.Ref) (signer.Generated, error) {
+	loaded, err := store.load(ctx, ref)
+	if err != nil {
+		return signer.Generated{}, err
+	}
+	defer loaded.Close()
+	// Both signers expose their public forms; the interface carries only what
+	// signing needs, so this asks for the rest.
+	type publisher interface {
+		Public() (signer.Generated, error)
+	}
+	public, ok := loaded.(publisher)
+	if !ok {
+		return signer.Generated{}, errors.New("signing key cannot produce public forms")
+	}
+	return public.Public()
+}
+
+func (store *Store) Resolve(ctx context.Context, ref signer.Ref) (signer.Signer, error) {
+	return store.load(ctx, ref)
+}
+
+func (store *Store) load(ctx context.Context, ref signer.Ref) (signer.Signer, error) {
+	name, err := keyName(ref)
+	if err != nil {
+		return nil, err
+	}
+	if err := ctx.Err(); err != nil {
+		return nil, err
+	}
+	content, err := store.material(name)
+	if err != nil {
+		return nil, err
+	}
+	defer wipe(content)
+	passphrase, err := store.passphrase()
+	if err != nil {
+		return nil, err
+	}
+	defer wipe(passphrase)
+	// The supplied form says which kind of key it is, exactly as the stored form
+	// does for the file backend, so a workspace holding both resolves each
+	// correctly without recording the algorithm a second time.
+	if bytes.Contains(content, []byte("SNAILMAIL ENCRYPTED APK PRIVATE KEY")) {
+		return apkrsa.Open(content, passphrase, signer.Identity{})
+	}
+	return openpgpsigner.Load(content, passphrase)
+}
+
+// material reads the key, preferring the file form.
+//
+// A container's environment is readable through the runtime's inspection API
+// for as long as the container exists, however the value was supplied, so a
+// mounted file that can be read once and removed is the better of the two. The
+// value form stays supported because it is what most CI secret integrations
+// offer directly. This mirrors how the passphrase itself is supplied.
+func (store *Store) material(name string) ([]byte, error) {
+	variable := Variable(name)
+	if path, present := store.lookup(variable + "_FILE"); present && path != "" {
+		if _, both := store.lookup(variable); both {
+			return nil, fmt.Errorf("set %s or %s, not both", variable, variable+"_FILE")
+		}
+		content, err := store.readFile(path)
+		if err != nil {
+			return nil, fmt.Errorf("read %s: %w", variable+"_FILE", err)
+		}
+		return bounded(bytes.TrimRight(content, "\r\n"), variable+"_FILE")
+	}
+	value, present := store.lookup(variable)
+	if !present || value == "" {
+		return nil, absent(variable)
+	}
+	return bounded([]byte(value), variable)
+}
+
+// absent reports a key the environment did not supply.
+//
+// It wraps ErrNotFound because callers branch on it -- `keys new` treats it as
+// "create one" -- but it names the variable, because the likeliest way to reach
+// it is a CI job that did not pass the secret, and "signing key not found" sends
+// whoever reads it looking in the wrong place.
+func absent(variable string) error {
+	return fmt.Errorf("%w: set %s or %s to the encrypted private key", signer.ErrNotFound, variable, variable+"_FILE")
+}
+
+func bounded(content []byte, source string) ([]byte, error) {
+	if len(content) == 0 {
+		return nil, absent(strings.TrimSuffix(source, "_FILE"))
+	}
+	if len(content) > maxMaterialBytes {
+		return nil, fmt.Errorf("%s exceeds %d bytes", source, maxMaterialBytes)
+	}
+	return content, nil
+}
+
+func keyName(ref signer.Ref) (string, error) {
+	if ref.Backend != Backend {
+		return "", errors.New("invalid env signing key reference")
+	}
+	match := referencePattern.FindStringSubmatch(ref.ID)
+	if match == nil {
+		return "", errors.New("invalid env signing key reference")
+	}
+	return match[2], nil
+}
+
+func wipe(value []byte) {
+	for index := range value {
+		value[index] = 0
+	}
+}