diff --git a/checks/sshdir/go.mod b/checks/sshdir/go.mod new file mode 100644 index 00000000..fb15b012 --- /dev/null +++ b/checks/sshdir/go.mod @@ -0,0 +1,3 @@ +module git.novox.be/novox/mesh-catalog/checks/sshdir + +go 1.22 diff --git a/checks/sshdir/main.go b/checks/sshdir/main.go new file mode 100644 index 00000000..0b7eb4ab --- /dev/null +++ b/checks/sshdir/main.go @@ -0,0 +1,112 @@ +// Command sshdir refuses any manifest but modules/ssh-client naming a path under ~/.ssh (novox/hq ADR 0415 §6, +// issue 1020): ssh-client owns ~/.ssh and sweeps what it does not know, so a path another module placed there +// would be moved out at the next sweep. A module that needs something there provides it to ssh-client instead. +// +// go run . +// +// Every string of a manifest is read, wherever it stands, except prose and a file's text (Prose): a path, a +// place, a volume. Exits 1 on a finding. +package main + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "sort" +) + +// SSHModule is the one module allowed to name a path under ~/.ssh. +const SSHModule = "ssh-client" + +// underSSH is a path under ~/.ssh, or ~/.ssh itself, however the home is spelled: ${machine:account-home}/.ssh, +// ~/.ssh, .ssh/config, or a volume's host side (…/.ssh:/root/.ssh). +var underSSH = regexp.MustCompile(`(^|[/~])\.ssh($|[/:])`) + +// Prose are the keys whose strings are words or a file's text, not a path: a template may say ~/.ssh in a comment. +var Prose = map[string]bool{"description": true, "why": true, "does": true, "template": true, "content": true, "title": true} + +// Finding is one path under ~/.ssh in a manifest other than ssh-client's. +type Finding struct { + Module string + Key string + Value string +} + +// Check reads every manifest under root/modules and answers which paths it refuses. +func Check(root string) ([]Finding, error) { + manifests, err := filepath.Glob(filepath.Join(root, "modules", "*", "module.json")) + if err != nil { + return nil, err + } + sort.Strings(manifests) + var out []Finding + for _, path := range manifests { + raw, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var m any + if err := json.Unmarshal(raw, &m); err != nil { + return nil, fmt.Errorf("%s: %w", path, err) + } + module := filepath.Base(filepath.Dir(path)) + if obj, ok := m.(map[string]any); ok { + if name, ok := obj["module"].(string); ok && name != "" { + module = name + } + } + if module == SSHModule { + continue + } + walk(m, "", func(key, value string) { + out = append(out, Finding{Module: module, Key: key, Value: value}) + }) + } + return out, nil +} + +// walk calls found for every string under v, outside Prose, that names a path under ~/.ssh. +func walk(v any, key string, found func(key, value string)) { + switch x := v.(type) { + case map[string]any: + keys := make([]string, 0, len(x)) + for k := range x { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + if !Prose[k] { + walk(x[k], k, found) + } + } + case []any: + for _, e := range x { + walk(e, key, found) + } + case string: + if underSSH.MatchString(x) { + found(key, x) + } + } +} + +func main() { + root := "." + if len(os.Args) > 1 { + root = os.Args[1] + } + found, err := Check(root) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + for _, f := range found { + fmt.Printf("%s names %q (%s) under ~/.ssh: only %s may, and the sweep moves anything else out (novox/hq ADR 0415 §6)\n", f.Module, f.Value, f.Key, SSHModule) + } + if len(found) > 0 { + os.Exit(1) + } + fmt.Println("~/.ssh: named only by ssh-client") +} diff --git a/checks/sshdir/main_test.go b/checks/sshdir/main_test.go new file mode 100644 index 00000000..ca78b3e7 --- /dev/null +++ b/checks/sshdir/main_test.go @@ -0,0 +1,48 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func manifest(t *testing.T, root, module, body string) { + t.Helper() + dir := filepath.Join(root, "modules", module) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "module.json"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +func TestAnotherModuleNamingSSHIsRefused(t *testing.T) { + root := t.TempDir() + manifest(t, root, "ssh-client", `{"module":"ssh-client","resources":[{"path":"${machine:account-home}/.ssh/config"}]}`) + manifest(t, root, "forge-account", `{"module":"forge-account","resources":[{"path":"${machine:account-home}/.ssh/config.d/20-forge"}]}`) + manifest(t, root, "roster", `{"module":"roster","facts":{"hosts":{"path":".ssh/known_hosts"}}}`) + manifest(t, root, "box", `{"module":"box","containers":[{"volumes":["~/.ssh:/root/.ssh:ro"]}]}`) + manifest(t, root, "quiet", `{"module":"quiet","description":"reads ~/.ssh/config","resources":[{"path":"${machine:account-home}/.sshrc-not","template":"# see ~/.ssh/config"},{"path":"/etc/ssh/sshd_config"}]}`) + found, err := Check(root) + if err != nil { + t.Fatal(err) + } + got := map[string]bool{} + for _, f := range found { + got[f.Module] = true + } + if len(found) != 3 || !got["forge-account"] || !got["roster"] || !got["box"] { + t.Fatalf("refused %+v; want forge-account, roster and box", found) + } +} + +func TestTheCatalogueHasNone(t *testing.T) { + found, err := Check("../..") + if err != nil { + t.Fatal(err) + } + if len(found) != 0 { + t.Fatalf("the catalogue names paths under ~/.ssh outside ssh-client: %+v", found) + } +} diff --git a/merge-check.sh b/merge-check.sh index 0e4e4cdc..8afe1cc1 100755 --- a/merge-check.sh +++ b/merge-check.sh @@ -45,6 +45,8 @@ # unless checks/templates/inline lists it as not yet moved; each names its template in files/. That list # only shrinks, judged against the trunk's copy at the merge base. # 10. the mail-store seat (novox/hq ADR 0378 §3, issue 800): no manifest but modules/mesh-mail claims it. +# 11. ~/.ssh (novox/hq ADR 0415 §6, issue 1020): no manifest but modules/ssh-client names a path under it — +# ssh-client owns it and sweeps out what it does not know. set -eu (cd checks/words && go test -count=1 . && go run . ../..) @@ -52,6 +54,8 @@ set -eu (cd checks/agenthome && go test -count=1 . && go run . ../..) # The mail-store seat (novox/hq ADR 0378 §3): only mesh-mail may claim it. (cd checks/mailstore && go test -count=1 . && go run . ../..) +# ~/.ssh (novox/hq ADR 0415 §6): only ssh-client may name a path under it. +(cd checks/sshdir && go test -count=1 . && go run . ../..) # The secrets (novox/hq issue 342): a password is not a setting, so no setting can replace the vault's. The # known list only shrinks: judged against the trunk's copy at the merge base, a line it does not hold is refused. secrets_base="" diff --git a/modules/ssh-client/README.md b/modules/ssh-client/README.md index 8b92fd9c..301cd579 100644 --- a/modules/ssh-client/README.md +++ b/modules/ssh-client/README.md @@ -8,13 +8,14 @@ item 9). `sshd` is the machine's side. | path | class | how | |---|---|---| | `~/.ssh/` | owned | directory, the account's, mode 0700 | -| `~/.ssh/config.d/` | owned | directory, the account's, mode 0700. It holds only the files this module declares (ADR 0415 §6). As every module's owned path, ssh-client is the only module that writes under `~/.ssh`: another module that needs a host there provides it through a provision, and this module writes it (ADR 0349 §1; the forge's host comes from `forge-account`). A host of the operator's own goes in `~/.ssh/config` below the mesh's region | +| `~/.ssh/config.d/` | owned | directory, the account's, mode 0700. Apart from keys, it holds only the files this module declares (ADR 0415 §7). As every module's owned path, ssh-client is the only module that writes under `~/.ssh`: another module that needs a host there provides it through a provision, and this module writes it (ADR 0349 §1; the forge's host comes from `forge-account`). A host of the operator's own goes in `~/.ssh/config` below the mesh's region | | `~/.ssh/config.d/10-forge` | owned | the forge's host, with the machine's own key (ADR 0399) | | `~/.ssh/mesh_ed25519`, `.pub` | written by the module's own process | the machine's account's key for the forge, made once (keykeeper.go) | | `~/.ssh/config.d/00-mesh` | owned | a Host block per other machine of the mesh (a roster fact), regenerated whenever a machine joins, leaves or is renamed. It begins with the mesh's header | | `~/.ssh/config` | written into, **at the start** | the mesh's region, `# BEGIN mesh ssh-client.config` … `# END …`. It holds one `Include ~/.ssh/config.d/*`. Every line below the region is the operator's, kept byte for byte | | `~/.ssh/authorized_keys` | found | see *The gap* | | `~/.ssh/known_hosts` | found | see *The gap* | +| `~/.ssh/authorized_keys2`, `known_hosts2`, `rc`, `environment` | kept | the files ssh and sshd read by themselves; this module writes none of them and never moves them (ADR 0415 §1, ADR 0417 §1) | | private keys, public keys, certificates | kept | never read, never written, never moved, at any depth and whatever their name. `ssh_client_keys` and `ssh_client_check` read a file's first line to recognise a key, and ask `ssh-keygen` about it | | anything else | swept | moved out of `~/.ssh` by the sweep (below) | @@ -68,6 +69,23 @@ of what moved is written beside it as `.json`. Nothing is deleted. A f is gone into rather than moved whole, so the key stays where it is. A file or folder that cannot be read to tell whether it holds a key is left and named in the log. +ADR 0417 (issue 1020) adds: + +- the files ssh and sshd read by themselves (`authorized_keys2`, `known_hosts2`, `rc`, `environment`) stay; +- a symbolic link to a key, or to a folder holding one, stays; a link whose target cannot be checked is left + and named; any other link is moved as the link; +- a path changed less than a minute ago is being written (ssh-keygen's or `ssh_client_revoke`'s temporary + file) and waits for the next sweep; +- the retired folder must be on `~/.ssh`'s file system: otherwise the sweep moves nothing, makes no + folder and fails. A sweep that moved nothing leaves no folder; +- the list names each move before it is made, and a list that cannot be written stops the sweep; +- each sweep writes what it did, or why it failed, to `~/.local/state/ssh-client/last-sweep.json`. + `ssh_client_check` reports a failed last sweep, and a listed move with nothing at its destination, as a + problem. + +No other module declares a path under `~/.ssh` (ADR 0415 §6): the catalogue's `checks/sshdir` fails one +that does. + So the predecessor's `config.d/mesh`, `config.bak-*`, `known_hosts.old`, `removed-*` and `authorized_keys.revoked-*` leave `~/.ssh` at the first sweep. Each move is logged by the process, and `ssh_client_check` lists every sweep that moved something, with where each path lies now. @@ -110,4 +128,7 @@ The tests use a temporary home and a fake runner, with public keys made for the - `known_host` matching, changed and refreshed, and an unreachable host never refreshed; - `test`: success, and failure with the agent named; - `check`'s findings; +- the sweep: what it keeps (keys at any depth, links to keys and to key folders, sockets, what ssh reads + itself), what it moves, what it leaves and names, a file being written, another file system, and the + list written before each move; - that the tools served are the manifest's `tools`. diff --git a/modules/ssh-client/cmd/ssh-client-tools/client.go b/modules/ssh-client/cmd/ssh-client-tools/client.go index 7e1398ad..454de9dd 100644 --- a/modules/ssh-client/cmd/ssh-client-tools/client.go +++ b/modules/ssh-client/cmd/ssh-client-tools/client.go @@ -8,6 +8,7 @@ import ( "bufio" "context" "fmt" + "io" "io/fs" "os" "path/filepath" @@ -24,6 +25,8 @@ type Client struct { UID int Run Runner Now func() time.Time + // Settle is how long a path in ~/.ssh is left by the sweep after it changed (sweep.go): zero in tests. + Settle time.Duration } // NewClient is the client the bundle serves with: the operator's home as the runtime names it. @@ -32,7 +35,7 @@ func NewClient() *Client { if home == "" { home, _ = os.UserHomeDir() } - return &Client{Home: home, UID: os.Getuid(), Run: ExecRunner, Now: time.Now} + return &Client{Home: home, UID: os.Getuid(), Run: ExecRunner, Now: time.Now, Settle: time.Minute} } func (c *Client) ssh(name string) string { return filepath.Join(c.Home, ".ssh", name) } @@ -173,7 +176,7 @@ func header(path string) bool { return false } defer f.Close() - line, _ := bufio.NewReader(f).ReadString('\n') + line, _ := bufio.NewReader(io.LimitReader(f, 256)).ReadString('\n') // a key's first line is short; a large file's may not be return strings.HasPrefix(line, "-----BEGIN") && strings.Contains(line, "PRIVATE KEY-----") } @@ -813,6 +816,22 @@ func (c *Client) Check(ctx context.Context, scan bool) (map[string]any, error) { if err != nil { add("problem", filepath.Join(c.Home, RetiredDir), "cannot be read: %v", err) } + for _, r := range retired { + for _, m := range r.Moved { + if m.Missing { + add("problem", m.To, "the sweep of %s lists %s as moved here, but nothing is here: it may still be in ~/.ssh", r.At, m.From) + } + } + } + last, err := c.LastSwept() + switch { + case err != nil: + add("problem", filepath.Join(c.Home, LastSweepFile), "cannot be read: %v", err) + case last == nil: + notChecked = append(notChecked, "the sweep: it has not run on this account yet") + case last.Error != "": + add("problem", dir, "the last sweep (%s) failed: %s", last.At, last.Error) + } problems := 0 for _, x := range f { if x.Severity == "problem" { diff --git a/modules/ssh-client/cmd/ssh-client-tools/ssh_test.go b/modules/ssh-client/cmd/ssh-client-tools/ssh_test.go index 3ceccc65..763540cb 100644 --- a/modules/ssh-client/cmd/ssh-client-tools/ssh_test.go +++ b/modules/ssh-client/cmd/ssh-client-tools/ssh_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "reflect" + "slices" "sort" "strings" "sync" @@ -376,6 +377,37 @@ func TestCheckFindsWhatIsWrongAndSaysWhatItDidNotCheck(t *testing.T) { } } +// Issue 1020, ADR 0417: the check shows a failed last sweep and a listed move with nothing at its destination +// as problems, and says when no sweep has run. +func TestCheckShowsAFailedSweep(t *testing.T) { + h := home(t, map[string]string{"config": layout["config"], "known_hosts": ""}) + f := &fake{answer: func(call) Ran { return Ran{Status: 1} }} + c := client(h, f) + got, _ := c.Check(context.Background(), false) + if !slices.ContainsFunc(got["not_checked"].([]string), func(s string) bool { return strings.Contains(s, "the sweep") }) { + t.Errorf("not_checked = %v, want the sweep named as not run yet", got["not_checked"]) + } + if err := c.writeLastSweep(LastSweep{At: "20261017T120000Z", Error: "it broke"}); err != nil { + t.Fatal(err) + } + list := filepath.Join(h, RetiredDir, "20261001T000000Z.json") + os.MkdirAll(filepath.Dir(list), 0o700) + os.WriteFile(list, []byte(`[{"from":"`+filepath.Join(h, ".ssh", "x")+`","to":"`+filepath.Join(h, RetiredDir, "20261001T000000Z", "x")+`"}]`), 0o600) + got, _ = c.Check(context.Background(), false) + var whats []string + for _, x := range got["findings"].([]Finding) { + if x.Severity == "problem" { + whats = append(whats, x.What) + } + } + all := strings.Join(whats, "\n") + for _, want := range []string{"the last sweep (20261017T120000Z) failed: it broke", "nothing is here"} { + if !strings.Contains(all, want) { + t.Errorf("missing problem %q in:\n%s", want, all) + } + } +} + func TestTheToolsServedAreTheToolsTheManifestNames(t *testing.T) { raw, err := os.ReadFile("../../module.json") if err != nil { diff --git a/modules/ssh-client/cmd/ssh-client-tools/sweep.go b/modules/ssh-client/cmd/ssh-client-tools/sweep.go index 539bcddb..c297bbf5 100644 --- a/modules/ssh-client/cmd/ssh-client-tools/sweep.go +++ b/modules/ssh-client/cmd/ssh-client-tools/sweep.go @@ -5,7 +5,10 @@ package main // keeping its path relative to ~/.ssh. Nothing is deleted. A key (private, public or a certificate), // at any depth and whatever its name, and authorized_keys are never moved (design 29 §3: moving one // can lock the operator out); a folder holding a key is gone into rather than moved whole. A socket -// is ssh's own runtime and stays. The process sweeps when it starts and every KeyRound (main.go). +// is ssh's own runtime and stays, and so do the files ssh and sshd read by themselves (SSHReads). A +// path changed in the last Settle is being written (a temporary file of atomicWrite or ssh-keygen) and +// waits for the next sweep. The process sweeps when it starts and every KeyRound (main.go); what the +// last sweep did, or why it failed, is kept beside the retired folder for ssh_client_check (issue 1020). import ( "encoding/json" @@ -16,6 +19,7 @@ import ( "regexp" "sort" "strings" + "syscall" "time" ) @@ -30,20 +34,38 @@ var Declared = map[string]bool{ KeyFile: true, KeyFile + ".pub": true, "authorized_keys": true, - "authorized_keys2": true, // sshd's second AuthorizedKeysFile by default: moving it can lock the operator out } +// SSHReads are the files ssh and sshd read in ~/.ssh by themselves, though this module writes none of +// them (ssh(1), sshd(8) FILES): moving one loses a trust or a login's setup, or locks the operator out. +var SSHReads = map[string]bool{ + "authorized_keys2": true, // sshd's second AuthorizedKeysFile by default + "known_hosts2": true, // ssh's second UserKnownHostsFile by default + "rc": true, // run by sshd at every login + "environment": true, // read by sshd at login when PermitUserEnvironment allows +} + +// known is whether rel, relative to ~/.ssh, is a path this module declares or one ssh reads itself. +func known(rel string) bool { return Declared[rel] || SSHReads[rel] } + // copyOf is a copy of authorized_keys or known_hosts (authorized_keys.revoked-*, known_hosts.old): it -// carries key lines but is no key, and sshd and ssh never read it. +// carries key lines but is no key, and sshd and ssh never read it. It is matched on a name, at any depth. var copyOf = regexp.MustCompile(`^(authorized_keys|known_hosts)[^/]+$`) // RetiredDir is where the sweep moves what it does not know, under the account's home. const RetiredDir = ".local/state/ssh-client/retired" +// LastSweepFile is where each sweep writes what it did, or why it failed, under the account's home: +// the sweep runs in the background, so a failure that is only logged is quiet (issue 1020). +const LastSweepFile = ".local/state/ssh-client/last-sweep.json" + // Moved is one path a sweep moved out of ~/.ssh. type Moved struct { From string `json:"from"` To string `json:"to"` + // Missing is set by Retired when nothing lies at To: the sweep listed the move before making it and + // stopped between the two. + Missing bool `json:"missing,omitempty"` } // Sweep is what one sweep did: what it moved, and what it left because it could not tell what it is. @@ -53,6 +75,14 @@ type Sweep struct { Left []string `json:"left,omitempty"` } +// LastSweep is what the last sweep did, as LastSweepFile holds it. +type LastSweep struct { + At string `json:"at"` + Moved int `json:"moved"` + Left []string `json:"left,omitempty"` + Error string `json:"error,omitempty"` +} + // Sweep moves every path under ~/.ssh this module does not know into a retired folder named by now, // and writes beside that folder the list of what it moved (