Merge pull request 'A login session is the account's own, of class user, read at accept (hq ADR 0272)' (#71) from feat/272-the-session-is-a-users-read-at-accept into main

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-<id>.scope` under the account's own `user-<uid>.slice`), or nothing; the controller makes
+session's scope (`session-<id>.scope` under the account's own `user-<uid>.slice`, which logind's own record in
+`/run/systemd/sessions/<id>` 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.
 

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}))
 }

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)
+	}
+}