diff --git a/README.md b/README.md index 63ded39..96ab198 100644 --- a/README.md +++ b/README.md @@ -160,7 +160,9 @@ connection's deadline set the moment it is accepted, two lines at once per accou "busy" beyond that. **Only a login session can be the terminal.** The engine reads the asking process's cgroup and sends its login -session's scope (`session-.scope` under the account's own `user-.slice`), or nothing; the controller makes +session's scope (`session-.scope` under the account's own `user-.slice`, which logind's own record in +`/run/systemd/sessions/` must say is this account's, of class `user`), or nothing, read the moment the +connection is accepted; the controller makes nothing else the terminal. A service running as the operator — the tool runner, a unit of the account's user manager — is at most an ordinary call. diff --git a/internal/meshcli/meshcli.go b/internal/meshcli/meshcli.go index 709fbb8..dc64264 100644 --- a/internal/meshcli/meshcli.go +++ b/internal/meshcli/meshcli.go @@ -96,7 +96,36 @@ var sessionOf = func(pid int32, uid uint32) string { if err != nil { return "" } - return loginSessionIn(string(raw), uid) + return userSessionIn(string(raw), uid) +} + +// sessionsDir is where logind keeps a record of each session it holds. A variable so a test can say. +var sessionsDir = "/run/systemd/sessions" + +// userSessionIn is the login session a cgroup names, when logind's own record of it says it is a person's: this +// account's (UID), of class `user` — not a greeter's or the user manager's — and of this very scope. The cgroup +// alone names a scope; only the record says whose session it is. No record, or one that does not say so, is no +// session. +func userSessionIn(cgroup string, uid uint32) string { + scope := loginSessionIn(cgroup, uid) + id, ok := strings.CutPrefix(strings.TrimSuffix(scope, ".scope"), "session-") + if !ok || id == "" || strings.ContainsAny(id, "/.") { + return "" + } + raw, err := os.ReadFile(filepath.Join(sessionsDir, id)) + if err != nil { + return "" + } + record := map[string]string{} + for _, line := range strings.Split(string(raw), "\n") { + if k, v, ok := strings.Cut(line, "="); ok { + record[k] = v + } + } + if record["UID"] != strconv.FormatUint(uint64(uid), 10) || record["CLASS"] != "user" || record["SCOPE"] != scope { + return "" + } + return scope } // loginSessionIn is the login session scope a process's cgroup names, for its own account: the unified hierarchy's @@ -219,6 +248,9 @@ func answer(ctx context.Context, conn *net.UnixConn, held *slots, forward Forwar "machine's account and nothing was asked: " + err.Error())) return } + // And the login session, now: judged as the peer was when it connected, before it can exec or exit while its + // line is read (review of ADR 0272). + session := sessionOf(pid, uid) if !held.take(uid) { reply(cli.Refusal(fmt.Sprintf("busy: user id %d has %d lines being asked already, or the machine %d; ask "+ "again when one has ended. Nothing was asked", uid, perAccount, atOnce))) @@ -239,5 +271,5 @@ func answer(ctx context.Context, conn *net.UnixConn, held *slots, forward Forwar _ = conn.SetDeadline(time.Now().Add(cli.AskWithin)) asking, cancel := context.WithTimeout(ctx, cli.AskWithin-30*time.Second) defer cancel() - reply(forward(asking, Asked{Line: req.Line, Account: account, UID: uid, Session: sessionOf(pid, uid)})) + reply(forward(asking, Asked{Line: req.Line, Account: account, UID: uid, Session: session})) } diff --git a/internal/meshcli/meshcli_test.go b/internal/meshcli/meshcli_test.go index 497d6bf..dd0f84f 100644 --- a/internal/meshcli/meshcli_test.go +++ b/internal/meshcli/meshcli_test.go @@ -249,7 +249,66 @@ func TestTheLoginSessionAskingIsReadFromTheKernel(t *testing.T) { t.Fatal(err) } raw, _ := os.ReadFile("/proc/self/cgroup") - if want := loginSessionIn(string(raw), uint32(os.Getuid())); got.Session != want { + if want := userSessionIn(string(raw), uint32(os.Getuid())); got.Session != want { t.Fatalf("sent session %q, the kernel says %q", got.Session, want) } } + +// **A login session is a person's: systemd's own record says so** (review of ADR 0272): the scope the cgroup names +// must be a session logind keeps for this very account, of class `user` — not a greeter's, not the user manager's, +// not one whose record names another account — and read at accept, with the namespaces, before anything else. +func TestALoginSessionIsAUsersByItsOwnRecord(t *testing.T) { + dir := t.TempDir() + was := sessionsDir + sessionsDir = dir + t.Cleanup(func() { sessionsDir = was }) + write := func(id, body string) { + if err := os.WriteFile(filepath.Join(dir, id), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + write("9", "UID=1000\nUSER=op\nCLASS=user\nSCOPE=session-9.scope\nACTIVE=1\n") + write("3", "UID=1000\nCLASS=greeter\nSCOPE=session-3.scope\n") + write("4", "UID=1001\nCLASS=user\nSCOPE=session-4.scope\n") + write("5", "UID=1000\nCLASS=user\nSCOPE=session-6.scope\n") + for _, c := range []struct { + cgroup string + uid uint32 + session string + }{ + {"0::/user.slice/user-1000.slice/session-9.scope\n", 1000, "session-9.scope"}, + {"0::/user.slice/user-1000.slice/x.scope\n", 1000, ""}, + {"0::/user.slice/user-1000.slice/session-9.scope\n", 1001, ""}, // a different uid than the scope's slice + {"0::/user.slice/user-1001.slice/session-4.scope\n", 1000, ""}, // another account's session + {"0::/user.slice/user-1000.slice/session-3.scope\n", 1000, ""}, // a greeter's + {"0::/user.slice/user-1000.slice/session-5.scope\n", 1000, ""}, // a record naming another scope + {"0::/user.slice/user-1000.slice/session-7.scope\n", 1000, ""}, // no record at all + {"0::/user.slice/user-1000.slice/session-../9.scope\n", 1000, ""}, + } { + if got := userSessionIn(c.cgroup, c.uid); got != c.session { + t.Errorf("%q as uid %d read as %q, want %q", c.cgroup, c.uid, got, c.session) + } + } +} + +// The session is read at accept, with the namespaces, and before the line: a peer that execs or exits while its +// line is read is judged as it was when it connected. +func TestTheSessionIsReadBeforeTheLine(t *testing.T) { + var order []string + beforeNS, beforeSession := sameNamespaces, sessionOf + sameNamespaces = func(int32) error { order = append(order, "namespaces"); return nil } + sessionOf = func(int32, uint32) string { order = append(order, "session"); return "session-1.scope" } + t.Cleanup(func() { sameNamespaces, sessionOf = beforeNS, beforeSession }) + var got Asked + socket := serving(t, func(_ context.Context, asked Asked) cli.Answer { + order = append(order, "forward") + got = asked + return cli.Answer{} + }) + if _, err := cli.Ask(context.Background(), socket, []string{"status"}); err != nil { + t.Fatal(err) + } + if strings.Join(order, ",") != "namespaces,session,forward" || got.Session != "session-1.scope" { + t.Fatalf("read in the order %v, sent %q", order, got.Session) + } +}