diff --git a/README.md b/README.md index d28d8c9a..02127526 100644 --- a/README.md +++ b/README.md @@ -229,6 +229,14 @@ Fenrir accepts remote commands through Unix sockets. The standard socket is echo "ls" | socat - UNIX-CLIENT:/tmp/fenrirscreenreader-daemon.sock ``` +The standard socket routes reader descendants to their ancestor reader. For +reattached terminal sessions or detached helpers, it matches the caller's +`WINDOWID` and `DISPLAY` to a unique terminal reader owned by the same user. +Requests from a Linux virtual console reach the console reader, subject to +its usual authorization. Missing or ambiguous identity produces a routing +error; a sole unrelated reader is never selected as a default. Instance +listing remains available even when routing is ambiguous. + Send commands to a private socket when targeting a particular reader. Examples using the standard socket: diff --git a/internal/control/coexistence_linux.go b/internal/control/coexistence_linux.go index 0a3f568e..9d0671fe 100644 --- a/internal/control/coexistence_linux.go +++ b/internal/control/coexistence_linux.go @@ -5,9 +5,13 @@ package control import ( "fmt" + "io" "os" "path/filepath" "slices" + "strconv" + "strings" + "syscall" ) func (s *Server) endpoint_snapshot() []endpoint { @@ -53,9 +57,9 @@ func (s *Server) private_path(data instance) string { return "" } -// Select a terminal only when it is an ancestor of the caller. Outside a -// terminal instance, prefer TTY; a single same-user terminal is unambiguous. -// Never silently send a command to an arbitrary terminal in a multi-reader session. +// Ancestry identifies ordinary PTY children. Reattached Screen sessions and +// detached helpers can lose that ancestry, so also use the caller's own X11 +// identity. Neither a single terminal nor a console is a safe default. func (s *Server) route_target(pid, uid int) (string, error) { instances := s.instances() var terminals, consoles []instance @@ -83,17 +87,81 @@ func (s *Server) route_target(pid, uid int) (string, error) { } } } - if len(consoles) == 1 { - return s.destination(consoles[0]) + identity := process_identity(pid, uid) + return s.route_identity(identity, uid, terminals, consoles) +} + +type processIdentity struct { + window, display string + console bool +} + +// Read only identity fields from the credentialed process, never a supplied PID +// or environment in the request text. Environment is a routing hint, not an +// authorization grant; the destination still checks the original SO_PEERCRED. +func process_identity(pid, uid int) processIdentity { + var identity processIdentity + info, err := os.Stat(fmt.Sprintf("/proc/%d", pid)) + if err != nil { + return identity } - if len(consoles) == 0 && len(terminals) == 1 { - return s.destination(terminals[0]) + if stat, ok := info.Sys().(*syscall.Stat_t); !ok || int(stat.Uid) != uid { + return identity } - if len(consoles) == 0 && len(terminals) == 0 { - // The application applies its normal authorization to the shared owner. - return "", nil + if data, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid)); err == nil { + end := strings.LastIndexByte(string(data), ')') + if end >= 0 { + fields := strings.Fields(string(data[end+1:])) + if len(fields) > 4 { + tty, _ := strconv.ParseInt(fields[4], 10, 64) + device := uint32(tty) + major := (device >> 8) & 0xfff + minor := (device & 0xff) | ((device >> 12) & 0xfff00) + identity.console = major == 4 && minor >= 1 && minor <= 63 + } + } } - return "", fmt.Errorf("caller does not identify a unique reader; use its private socket") + file, err := os.Open(fmt.Sprintf("/proc/%d/environ", pid)) + if err != nil { + return identity + } + defer file.Close() + data, err := io.ReadAll(io.LimitReader(file, 1<<20)) + if err != nil { + return identity + } + for _, entry := range strings.Split(string(data), "\x00") { + key, value, _ := strings.Cut(entry, "=") + switch key { + case "WINDOWID": + identity.window = value + case "DISPLAY": + identity.display = value + } + } + return identity +} + +func (s *Server) route_identity(identity processIdentity, uid int, terminals, consoles []instance) (string, error) { + var matches []instance + if identity.console { + // A real controlling VT takes precedence over inherited X11 variables. + matches = consoles + } else if identity.window != "" && identity.display != "" { + for _, data := range terminals { + if data.uid != uid || data.WindowID != identity.window { + continue + } + target := process_identity(data.PID, data.uid) + if target.display == identity.display { + matches = append(matches, data) + } + } + } + if len(matches) == 1 { + return s.destination(matches[0]) + } + return "", fmt.Errorf("caller does not identify a unique reader; run inside its managed terminal or use its private socket") } func (s *Server) destination(data instance) (string, error) { diff --git a/internal/control/coexistence_test.go b/internal/control/coexistence_test.go index 24b6abc4..203c8996 100644 --- a/internal/control/coexistence_test.go +++ b/internal/control/coexistence_test.go @@ -72,7 +72,7 @@ func child_command(mode string) *exec.Cmd { func run_reader_process(mode string) { encoder := json.NewEncoder(os.Stdout) directory := os.Getenv("FENRIR_CONTROL_DIRECTORY") - server, err := Open(context.Background(), Options{Directory: directory, RegistryDirectory: filepath.Join(directory, "registry"), TerminalMode: mode == "terminal"}) + server, err := Open(context.Background(), Options{Directory: directory, RegistryDirectory: filepath.Join(directory, "registry"), TerminalMode: mode == "terminal", WindowID: os.Getenv("WINDOWID")}) if err != nil { _ = encoder.Encode(processReply{Error: err.Error()}) return @@ -239,7 +239,7 @@ func TestReadersCoexistInBothStartupOrdersAndSurviveOwnerExit(t *testing.T) { privatePath := filepath.Join(options.Directory, fmt.Sprintf("fenrirscreenreader-%d.sock", reader.pid)) check_receipt(t, second.send(t, privatePath, "COMMAND SAY private"), reader.pid, "COMMAND SAY private") } - // No descendant terminal: a legacy caller belongs to the TTY reader. + // An unrelated caller must not fall through to the console reader. client := child_command("client") client.Env = append(client.Env, "FENRIR_CONTROL_PATH="+mainPath, "FENRIR_CONTROL_TEXT=COMMAND SAY legacy") data, err := client.Output() @@ -250,7 +250,9 @@ func TestReadersCoexistInBothStartupOrdersAndSurviveOwnerExit(t *testing.T) { if err := json.Unmarshal(data, &reply); err != nil { t.Fatal(err) } - check_receipt(t, reply, tty.pid, "COMMAND SAY legacy") + if !strings.Contains(reply.Response, "unique reader") { + t.Fatal("unidentified caller reached a reader", reply) + } owner.stop(t) wait_main_owner(t, observer, 2, owner.pid) for _, reader := range []*readerProcess{tty, first, second} { @@ -328,3 +330,89 @@ func TestTerminalReadersRejectAmbiguityAndRecoverAfterOwnerFailure(t *testing.T) check_receipt(t, reader.send(t, mainPath, "COMMAND SAY recovered"), reader.pid, "COMMAND SAY recovered") } } + +// Screen reattachment and Codex's app-server can leave the caller outside the +// reader's descendant tree. The connecting process retains its X11 identity. +func TestWindowRoutingWithoutReaderAncestor(t *testing.T) { + for _, consoleFirst := range []bool{true, false} { + name := "terminal-first" + if consoleFirst { + name = "console-first" + } + t.Run(name, func(t *testing.T) { + options := private_options(t) + observer := &Server{ctx: context.Background(), options: options} + mainPath := observer.main_path() + t.Setenv("DISPLAY", ":fixture") + t.Setenv("WINDOWID", "42") + var terminal *readerProcess + if consoleFirst { + start_reader_process(t, options.Directory, "tty") + } + terminal = start_reader_process(t, options.Directory, "terminal") + if !consoleFirst { + start_reader_process(t, options.Directory, "tty") + } + // This client is a sibling, not a child, of the terminal reader. + text := "command application codex" + check_receipt(t, external_request(t, mainPath, text), terminal.pid, text) + t.Setenv("WINDOWID", "43") + second := start_reader_process(t, options.Directory, "terminal") + check_receipt(t, external_request(t, mainPath, text), second.pid, text) + t.Setenv("WINDOWID", "42") + check_receipt(t, external_request(t, mainPath, text), terminal.pid, text) + // Even a sole terminal on a different display is not a match. + t.Setenv("DISPLAY", ":other") + if reply := external_request(t, mainPath, text); !strings.Contains(reply.Response, "unique reader") { + t.Fatal("cross-display routing", reply) + } + t.Setenv("DISPLAY", ":fixture") + t.Setenv("WINDOWID", "unknown") + if reply := external_request(t, mainPath, text); !strings.Contains(reply.Response, "unique reader") { + t.Fatal("unknown window routed", reply) + } + t.Setenv("WINDOWID", "") + if reply := external_request(t, mainPath, text); !strings.Contains(reply.Response, "unique reader") { + t.Fatal("unidentified background caller routed", reply) + } + t.Setenv("WINDOWID", "42") + start_reader_process(t, options.Directory, "terminal") + if reply := external_request(t, mainPath, text); !strings.Contains(reply.Response, "unique reader") { + t.Fatal("duplicate window routed arbitrarily", reply) + } + // Discovery remains usable when routing is ambiguous. + if reply := external_request(t, mainPath, "ls"); !strings.Contains(reply.Response, "pid=") { + t.Fatal("discovery failed", reply) + } + }) + } +} + +func TestConsoleIdentityAndUnknownCallers(t *testing.T) { + options := private_options(t) + console := instance{PID: os.Getpid(), uid: os.Geteuid(), ScreenDriver: "vcsaDriver"} + server := &Server{ctx: context.Background(), options: options} + for _, trial := range []struct { + name string + identity processIdentity + consoles []instance + valid bool + }{ + {"console", processIdentity{console: true}, []instance{console}, true}, + {"console ignores stale window", processIdentity{console: true, window: "42", display: ":fixture"}, []instance{console}, true}, + {"multiple consoles", processIdentity{console: true}, []instance{console, console}, false}, + {"no console", processIdentity{console: true}, nil, false}, + {"background", processIdentity{}, []instance{console}, false}, + {"unknown window", processIdentity{window: "42", display: ":fixture"}, []instance{console}, false}, + } { + t.Run(trial.name, func(t *testing.T) { + _, err := server.route_identity(trial.identity, os.Geteuid(), nil, trial.consoles) + if (err == nil) != trial.valid { + t.Fatal("unexpected routing", err) + } + }) + } + if got := process_identity(os.Getpid(), os.Geteuid()+1); got != (processIdentity{}) { + t.Fatal("read identity of a different UID", got) + } +} diff --git a/internal/control/server_test.go b/internal/control/server_test.go index d08c4709..3fdc668c 100644 --- a/internal/control/server_test.go +++ b/internal/control/server_test.go @@ -262,7 +262,7 @@ func TestStaleSocketReplacementAndPrivateInstanceSelection(t *testing.T) { t.Fatal("registered ancestor instance not selected", target, err) } target, err = server.route_target(pid, os.Geteuid()+1) - if err != nil || target != "" { + if err == nil || target != "" { t.Fatal("forwarded to another user's instance", target, err) } }