From bf3f45e76902cfc991c9200cf2bc9a11151877ae Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 13:55:45 +0200 Subject: [PATCH] A login session is the account's own, of class user, read at accept (hq ADR 0272) The cgroup names a scope; only logind's record of it says whose session it is. The engine now takes a session only when /run/systemd/sessions/ says it is this account's (UID), of class user (not a greeter's or the user manager's), and of this very scope. And it reads the session the moment the connection is accepted, with the namespaces, before the line, so the peer is judged as it was when it connected. Tests: session-9.scope with a user record is taken; x.scope, a different uid, another account's session, a greeter's, a record naming another scope, no record, and a path with dots are not; the session is read before the line is forwarded. --- README.md | 4 ++- internal/meshcli/meshcli.go | 36 +++++++++++++++++-- internal/meshcli/meshcli_test.go | 61 +++++++++++++++++++++++++++++++- 3 files changed, 97 insertions(+), 4 deletions(-) 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) + } +}