diff --git a/cmd/cli/gateway.go b/cmd/cli/gateway.go new file mode 100644 index 0000000..7d6cc2f --- /dev/null +++ b/cmd/cli/gateway.go @@ -0,0 +1,235 @@ +package cli + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/qubesome/cli/internal/files" + "github.com/qubesome/cli/internal/gateway" + "github.com/qubesome/cli/internal/session" + "github.com/qubesome/cli/internal/types" + "github.com/urfave/cli/v3" +) + +// gatewayCommand reports on the session's gateway and takes it down. +// +// It is hidden, but not for the reason supervise and session-hold are. +// Nothing here runs inside a sandbox. It is hidden because qubesome manages +// the gateway itself: a launch starts one, reuses the one it finds and asks +// it to re-read its policy, so the ordinary way to get a gateway is to run a +// workload and there is nothing for a user to do here. What is left is an +// operator's business, and the one thing a launch will not do is replace a +// running gateway with one from a different image. +// +// Neither subcommand takes a profile. There is one gateway per session and +// not one per profile, because the policy it applies is keyed by workload +// across every profile. +var ( + logsFollow bool + logsLast int + logsWorkload string +) + +func gatewayCommand() *cli.Command { + cmd := &cli.Command{ + Name: "gateway", + Hidden: true, + Usage: "inspects and stops the session gateway", + Description: `qubesome starts, reuses and reloads the session gateway on its own, +so this is for the cases where that is not enough: + +qubesome gateway status - Report what qubesome knows about the session's gateway +qubesome gateway stop - Stop the session's gateway, leaving the session itself up +qubesome gateway logs - Show what the session's gateway has said + +A running gateway is reused whatever image it came from, so a change to +the gateway block of the config reaches nothing until it is stopped. The +next launch then starts a fresh one inside the same session. +`, + Commands: []*cli.Command{ + gatewayStatusCommand(), + gatewayStopCommand(), + gatewayLogsCommand(), + }, + } + return cmd +} + +func gatewayStatusCommand() *cli.Command { + return &cli.Command{ + Name: "status", + Usage: "reports what qubesome knows about the session gateway", + Action: func(ctx context.Context, cmd *cli.Command) error { + g := gateway.Current() + + cfg, problem := sessionConfig() + + // Inspect reports that it could not tell rather than leaving + // the lines blank, so a config that could not be identified + // is passed on as none with the reason it could not. + status := g.Inspect(session.Current(), cfg, g.StatusReady) + if problem != "" { + status.ConfigProblem = problem + } + + return status.Write(os.Stdout) + }, + } +} + +// sessionConfig returns the config describing this session's gateway, and +// why it could not be told when it cannot. +// +// Not profileConfigOrDefault. That falls back to the user-level config as +// soon as more than one profile is active, and the gateway is session +// wide: it was started by whichever launch found none running, from that +// profile's config, and may have come from any of them. Reporting the +// user-level file's gateway block for it would name an image, a policy and +// a subnet that the running gateway need not have anything to do with. +// +// Several active profiles are usually not an ambiguity at all, because one +// qubesome config commonly defines several profiles and they were all +// started from the same file. It is only profiles started from different +// files that leave nothing here able to say which one the gateway came +// from, and then saying so is the answer. +func sessionConfig() (*types.Config, string) { + active := activeConfigs() + + resolved := make([]string, 0, len(active)) + for _, path := range active { + // The run dir holds a symlink per active profile, so two profiles + // sharing a config are two links to one file. + target, err := filepath.EvalSymlinks(path) + if err != nil { + continue + } + resolved = append(resolved, target) + } + + if path, ok := sessionConfigPath(resolved); ok { + if cfg := config(path); cfg != nil && len(cfg.Profiles) > 0 { + return cfg, "" + } + } + + if len(resolved) > 1 { + return nil, fmt.Sprintf( + "%d profiles are active and were started from different configs, so which of them the "+ + "running gateway came from cannot be told, and the image, policy and subnet it "+ + "names are unknown", len(resolved)) + } + + // No profile running, or one whose config no longer reads. The + // user-level file is the only thing left that describes a gateway. + return profileConfigOrDefault(""), "" +} + +// sessionConfigPath returns the one config file every active profile was +// started from, and whether there was one. +func sessionConfigPath(active []string) (string, bool) { + if len(active) == 0 { + return "", false + } + + for _, path := range active[1:] { + if path != active[0] { + return "", false + } + } + + return active[0], true +} + +func gatewayStopCommand() *cli.Command { + return &cli.Command{ + Name: "stop", + Usage: "stops the session gateway, leaving the session itself up", + Action: func(ctx context.Context, cmd *cli.Command) error { + pid, err := gateway.Current().Stop() + if err != nil { + return err + } + + if pid == 0 { + fmt.Fprintln(os.Stdout, "no gateway is running for this session") + return nil + } + + fmt.Fprintf(os.Stdout, + "stopped the session gateway, pid %d. The session is still up, "+ + "so the next launch starts a fresh gateway inside it.\n", pid) + + return nil + }, + } +} + +// gatewayLogsCommand shows what the gateway has said. +// +// The gateway is started by whichever qubesome run found none running, and +// it is put in a session of its own so that a Ctrl-C at that terminal does +// not take the session's egress with it. Its output has nowhere to go that +// anybody is still watching, so it is written to a file, and this is how it +// is read back. It is the record of which host a workload was allowed or +// refused, which is the one thing needed when a workload cannot reach +// something it should. +func gatewayLogsCommand() *cli.Command { + return &cli.Command{ + Name: "logs", + Usage: "show the session gateway's logs", + Description: `Examples: + +qubesome gateway logs - Print the log of the gateway this session is running +qubesome gateway logs -n 50 - Print its last 50 lines +qubesome gateway logs -f - Print it and keep printing what is added +qubesome gateway logs -profile work - Only the lines about that profile's workloads +qubesome gateway logs -workload chrome - Only the lines about that workload, in any profile +qubesome gateway logs -profile work -workload chrome + - Only the lines about that one workload + +The log covers the gateway that is running. Starting a gateway begins it +afresh, so there is nothing here for a session that has not started one. + +The gateway knows a workload as its name and its profile's joined by a +dash, and either half may hold a dash of its own, so naming only one of +the two matches the other loosely. Naming both is exact. A line about no +workload, such as the gateway's own startup, is not shown when either +filter is given. +`, + Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: "follow", + Aliases: []string{"f"}, + Usage: "keep printing what is added to the log", + Destination: &logsFollow, + }, + &cli.StringFlag{ + Name: "profile", + Usage: "only the lines about the workloads of this profile", + Destination: &targetProfile, + }, + &cli.StringFlag{ + Name: "workload", + Usage: "only the lines about this workload", + Destination: &logsWorkload, + }, + &cli.IntFlag{ + Name: "lines", + Aliases: []string{"n"}, + Usage: "print only this many of the log's last matching lines", + Destination: &logsLast, + }, + }, + Action: func(ctx context.Context, _ *cli.Command) error { + return gateway.ShowLogs(ctx, os.Stdout, gateway.LogOptions{ + Path: files.GatewayLogPath(), + Profile: targetProfile, + Workload: logsWorkload, + Last: logsLast, + Follow: logsFollow, + }) + }, + } +} diff --git a/cmd/cli/gateway_test.go b/cmd/cli/gateway_test.go new file mode 100644 index 0000000..1dee4f7 --- /dev/null +++ b/cmd/cli/gateway_test.go @@ -0,0 +1,60 @@ +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// The gateway belongs to the session and not to a profile, so its status +// has no profile to be told which config to read. Every active profile +// naming the same file is the ordinary case, since one qubesome config +// usually defines several profiles, and that file is the answer. Two +// profiles started from different files is the case that has no answer. +func TestSessionConfigPath(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + active []string + want string + wantOne bool + }{ + { + name: "nothing active", + active: nil, + }, + { + name: "one profile", + active: []string{"/home/u/dotfiles/qubesome.yaml"}, + want: "/home/u/dotfiles/qubesome.yaml", + wantOne: true, + }, + { + name: "several profiles from one config", + active: []string{ + "/home/u/dotfiles/qubesome.yaml", + "/home/u/dotfiles/qubesome.yaml", + }, + want: "/home/u/dotfiles/qubesome.yaml", + wantOne: true, + }, + { + name: "profiles from different configs", + active: []string{ + "/home/u/dotfiles/qubesome.yaml", + "/home/u/work/qubesome.yaml", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, ok := sessionConfigPath(tc.active) + assert.Equal(t, tc.wantOne, ok) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/cmd/cli/host_run.go b/cmd/cli/host_run.go index c2c8007..4a7db62 100644 --- a/cmd/cli/host_run.go +++ b/cmd/cli/host_run.go @@ -3,11 +3,91 @@ package cli import ( "context" "fmt" + "os" "os/exec" + "slices" + "strconv" + "strings" + "github.com/qubesome/cli/internal/files" "github.com/urfave/cli/v3" ) +// hostRunEnv returns the environment for a command the host runs on a +// profile's display. +// +// The host environment is inherited rather than replaced. The command runs +// on the host as the user, with the user's own privileges, so withholding +// anything from it isolates nothing. Starting from an empty environment +// only takes away HOME and PATH, without which most host applications +// cannot find their own configuration, or a shell to run. +// +// DISPLAY and XAUTHORITY then point it at the profile instead of the host +// session. The cookie is the one the profile's workloads authenticate with, +// and the profile's X server refuses a connection that arrives without it, +// saying only that no authorization protocol was specified. +// +// What is dropped is the set of variables naming the session the command +// was launched from, which is not the session it is about to appear in. +// See launchingSession. +// +// The two entries are appended rather than substituted for the inherited +// ones. os/exec keeps the last value of a repeated key, so these are the +// values the command reads. +func hostRunEnv(base []string, display uint8, cookie string) []string { + env := make([]string, 0, len(base)+2) + for _, e := range base { + if namesTheLaunchingSession(e) { + continue + } + env = append(env, e) + } + + return append(env, + "DISPLAY=:"+strconv.Itoa(int(display)), + "XAUTHORITY="+cookie, + ) +} + +// launchingSession are the variables describing the display session the +// command was typed or bound in, rather than the profile it is being sent +// to. Each one is a handle on the host session, and the command is about +// to connect to a different display server, so each is either ignored +// there or acted on as if it meant something. +// +// WAYLAND_DISPLAY is a path to the host compositor. A toolkit that finds +// one connects to it and ignores DISPLAY, opening the window on the host +// desktop rather than in the profile. The profile's own window manager is +// started without it for the same reason. +// +// DESKTOP_STARTUP_ID is an X11 startup notification handed out by +// whatever launched qubesome. A window manager that spawns through +// startup notification records the workspace it spawned from against that +// id, and the application exports it to the next window it opens as +// _NET_STARTUP_ID. The profile's window manager then reads an id for a +// launch it never saw, and places the window by what that resolves to +// rather than on the workspace being looked at. It is also single use: it +// belongs to the launch that created it and to no later one. +// +// XDG_ACTIVATION_TOKEN is the Wayland spelling of the same thing, with +// the same two problems. +var launchingSession = []string{ + "WAYLAND_DISPLAY", + "DESKTOP_STARTUP_ID", + "XDG_ACTIVATION_TOKEN", +} + +// namesTheLaunchingSession reports whether an environment entry is one of +// launchingSession. +func namesTheLaunchingSession(entry string) bool { + name, _, ok := strings.Cut(entry, "=") + if !ok { + return false + } + + return slices.Contains(launchingSession, name) +} + func hostRunCommand() *cli.Command { cmd := &cli.Command{ Name: "host-run", @@ -37,12 +117,28 @@ qubesome host-run -profile firefox - Run firefox on the host and d return err } + cookie, err := files.ClientCookiePath(prof.Name) + if err != nil { + return err + } + c := exec.Command(commandName, cmd.Args().Slice()...) //nolint - c.Env = append(c.Env, fmt.Sprintf("DISPLAY=:%d", prof.Display)) - out, err := c.CombinedOutput() - fmt.Println(string(out)) + c.Env = hostRunEnv(os.Environ(), prof.Display, cookie) + + // This returns while the command keeps running, as launching + // a workload does, so the terminal it was typed at is free + // again. Its stdin is left closed rather than pointed at that + // terminal, which the shell has taken back. Its output still + // goes there, because a command that fails to reach the + // profile's display says why on it. + c.Stdout = os.Stdout + c.Stderr = os.Stderr + + if err := c.Start(); err != nil { + return fmt.Errorf("failed to start %q: %w", commandName, err) + } - return err + return nil }, } return cmd diff --git a/cmd/cli/host_run_test.go b/cmd/cli/host_run_test.go new file mode 100644 index 0000000..7558ed4 --- /dev/null +++ b/cmd/cli/host_run_test.go @@ -0,0 +1,71 @@ +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHostRunEnv(t *testing.T) { + t.Parallel() + + got := hostRunEnv([]string{ + "HOME=/home/user", + "PATH=/usr/bin", + "DISPLAY=:0", + "XAUTHORITY=/home/user/.Xauthority", + }, 21, "/home/user/.qubesome/run/work/.Xclient-cookie") + + require.Equal(t, []string{ + "HOME=/home/user", + "PATH=/usr/bin", + "DISPLAY=:0", + "XAUTHORITY=/home/user/.Xauthority", + "DISPLAY=:21", + "XAUTHORITY=/home/user/.qubesome/run/work/.Xclient-cookie", + }, got) +} + +func TestHostRunEnvDropsWaylandDisplay(t *testing.T) { + t.Parallel() + + got := hostRunEnv([]string{ + "HOME=/home/user", + "WAYLAND_DISPLAY=wayland-0", + }, 21, "/cookie") + + require.NotContains(t, got, "WAYLAND_DISPLAY=wayland-0") + require.Contains(t, got, "HOME=/home/user") +} + +// A window manager that spawns through startup notification records the +// workspace it spawned from against the id it exports. Carrying that id +// into another display server hands the profile's window manager a +// sequence it never started, and the window lands wherever that resolves +// to instead of where the user is looking. +func TestHostRunEnvDropsTheLaunchingSession(t *testing.T) { + t.Parallel() + + got := hostRunEnv([]string{ + "HOME=/home/user", + "PATH=/usr/bin", + "DESKTOP_STARTUP_ID=host/awesome/1-2-3_TIME12345", + "XDG_ACTIVATION_TOKEN=abcdef", + "WAYLAND_DISPLAY=wayland-0", + }, 21, "/cookie") + + for _, unwanted := range []string{ + "DESKTOP_STARTUP_ID", + "XDG_ACTIVATION_TOKEN", + "WAYLAND_DISPLAY", + } { + for _, e := range got { + assert.NotContains(t, e, unwanted+"=", + "%s names the session the command was launched from", unwanted) + } + } + + assert.Contains(t, got, "HOME=/home/user", "the host environment is otherwise kept") + assert.Contains(t, got, "PATH=/usr/bin") +} diff --git a/cmd/cli/root.go b/cmd/cli/root.go index 66f728e..fcb9af6 100644 --- a/cmd/cli/root.go +++ b/cmd/cli/root.go @@ -48,6 +48,7 @@ func RootCommand() *cli.Command { gpuCommand(), superviseCommand(), sessionHoldCommand(), + gatewayCommand(), vmInitCommand(), consoleCommand(), }, diff --git a/internal/doctor/env.go b/internal/doctor/env.go index 936c0b6..c6f47d1 100644 --- a/internal/doctor/env.go +++ b/internal/doctor/env.go @@ -7,6 +7,7 @@ import ( "strconv" "time" + "github.com/qubesome/cli/internal/files" "github.com/qubesome/cli/internal/gateway" "github.com/qubesome/cli/internal/images" "github.com/qubesome/cli/internal/runners/util/usb" @@ -85,6 +86,15 @@ type Env interface { // would call a gateway ready while a workload started against it // would still have egress with no rules on it. GatewayReady() error + + // GatewayLog reports what the session gateway's log says has + // happened: how many connections it classified, how many it refused + // and to where, and whether anything failed. + // + // A summary rather than the log itself, because the log's shape is + // the gateway's own and reading it belongs in the one place that + // already understands it. + GatewayLog() (gateway.LogSummary, error) } // OSEnv is the real host. @@ -175,6 +185,15 @@ func (e *OSEnv) GatewayReady() error { return c.Ready(ctx) } +// GatewayLog reads the log the session's gateway writes. +// +// Nothing is asked of the gateway for this. It is the record the launch +// left behind, so it answers for a gateway that has stopped talking as +// well as for one that is well. +func (e *OSEnv) GatewayLog() (gateway.LogSummary, error) { + return gateway.Summarise(files.GatewayLogPath()) +} + func contextWithTimeout(d time.Duration) (context.Context, context.CancelFunc) { if d <= 0 { return context.WithCancel(context.Background()) diff --git a/internal/doctor/environment_test.go b/internal/doctor/environment_test.go index 24c7bc8..edc863e 100644 --- a/internal/doctor/environment_test.go +++ b/internal/doctor/environment_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/qubesome/cli/internal/files" + "github.com/qubesome/cli/internal/gateway" "github.com/stretchr/testify/require" ) @@ -29,6 +30,8 @@ type fakeEnv struct { // error is a ready gateway, which is also the zero value, so only a // test that wants a failure has to set it. gatewayReady error + log gateway.LogSummary + logErr error } type fakeOutput struct { @@ -128,6 +131,12 @@ func (f *fakeEnv) GatewayReady() error { return f.gatewayReady } +// GatewayLog answers with a canned summary, so a test can drive a check +// on what a gateway log said without writing one. +func (f *fakeEnv) GatewayLog() (gateway.LogSummary, error) { + return f.log, f.logErr +} + type fakeFileInfo struct { name string isDir bool diff --git a/internal/doctor/session.go b/internal/doctor/session.go index 033231a..d85e6e6 100644 --- a/internal/doctor/session.go +++ b/internal/doctor/session.go @@ -2,6 +2,7 @@ package doctor import ( "fmt" + "strings" "github.com/qubesome/cli/internal/files" "github.com/qubesome/cli/internal/types" @@ -33,10 +34,10 @@ func Session(env Env, cfg *types.Config) []Check { checks := []Check{holder, gateway} if gateway.Status == OK { - // Readiness only says something once there is a gateway to ask. - // Piling a second failure on top of the same cause would bury - // the one worth reading. - checks = append(checks, checkGatewayReady(env)) + // Readiness and egress only say something once there is a + // gateway to ask about. Piling a second failure on top of the + // same cause would bury the one worth reading. + checks = append(checks, checkGatewayReady(env), checkGatewayEgress(env)) } return checks @@ -165,8 +166,8 @@ func checkGatewayReady(env Env) Check { Name: name, Status: Fail, Detail: fmt.Sprintf("the gateway is running but did not report itself ready: %s", firstLine(err.Error())), - Fix: "Its resolver, proxy or netfilter ruleset did not come up. Check the gateway's own " + - "output, and the policy file the gateway block names.", + Fix: "Its resolver, proxy or netfilter ruleset did not come up. Read `qubesome gateway " + + "logs` for what it said, and check the policy file the gateway block names.", } } @@ -176,3 +177,68 @@ func checkGatewayReady(env Env) Check { Detail: "the gateway reports its resolver, proxy and ruleset are up", } } + +// checkGatewayEgress reports what the gateway has been doing with the +// connections its workloads made. +// +// A refused connection and a failed one are not the same finding. A +// denial is the policy doing what it says, so it is reported without +// being called a fault, and the hosts are named because "why can I not +// reach this" is what brings someone here. An error is something the +// gateway tried to do and could not, which is worth a warning. +func checkGatewayEgress(env Env) Check { + const name = "gateway egress" + + summary, err := env.GatewayLog() + if err != nil { + return Check{ + Name: name, + Status: Warn, + Detail: fmt.Sprintf("the gateway is running but what it has said cannot be read: %s", + firstLine(err.Error())), + Fix: "A gateway started before qubesome kept a log has none. Restart the session to " + + "get one, or read the terminal the gateway was started from.", + } + } + + if summary.Errors > 0 { + return Check{ + Name: name, + Status: Warn, + Detail: fmt.Sprintf("the gateway reported %s, most recently %q", + plural(summary.Errors, "error"), summary.LastError), + Fix: "Read `qubesome gateway logs` for the whole of it. `-workload` and `-profile` " + + "narrow it to one workload.", + } + } + + if summary.Denied == 0 { + return Check{ + Name: name, + Status: OK, + Detail: fmt.Sprintf("the gateway classified %s and refused none", + plural(summary.Decisions, "connection")), + } + } + + return Check{ + Name: name, + Status: OK, + Detail: fmt.Sprintf("the gateway classified %s and refused %d, to %s", + plural(summary.Decisions, "connection"), + summary.Denied, strings.Join(summary.DeniedHosts, ", ")), + Fix: "This is the policy being applied, and is only a problem if one of those hosts was " + + "meant to be reachable. A workload reaches a host named under egress.allowed or " + + "dns.allowed for it, and the gateway mirrors one onto the other when only one is set.", + } +} + +// plural renders a count with its noun, so a report reads as a sentence +// rather than as a field. +func plural(n int, noun string) string { + if n == 1 { + return "1 " + noun + } + + return fmt.Sprintf("%d %ss", n, noun) +} diff --git a/internal/doctor/session_test.go b/internal/doctor/session_test.go index 45f8ca0..0adb0f6 100644 --- a/internal/doctor/session_test.go +++ b/internal/doctor/session_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/qubesome/cli/internal/files" + "github.com/qubesome/cli/internal/gateway" "github.com/qubesome/cli/internal/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -171,3 +172,96 @@ func TestRunReportsTheSessionWithoutAProfile(t *testing.T) { checks := sectionByPrefix(t, report, "Session") assert.Equal(t, OK, checkByName(t, checks, "session gateway").Status) } + +// A denial is the policy working, so it is reported without being called +// a fault. It is also the answer to why a workload cannot reach +// something, which is what brings someone to doctor in the first place, +// so the hosts are named. +func TestSessionReportsDeniedHosts(t *testing.T) { + t.Parallel() + + env := runningSession() + env.log = gateway.LogSummary{ + Decisions: 9, + Denied: 2, + DeniedHosts: []string{"ads.example.com", "telemetry.example.com"}, + } + + check := egressCheck(t, Session(env, gatewayConfig())) + + assert.Equal(t, OK, check.Status, "a policy refusing a host is not a broken gateway") + assert.Contains(t, check.Detail, "2") + assert.Contains(t, check.Detail, "ads.example.com") + assert.Contains(t, check.Detail, "telemetry.example.com") +} + +// An error is not a denial. Something the gateway tried to do did not +// work, and that is worth warning about. +func TestSessionWarnsOnGatewayErrors(t *testing.T) { + t.Parallel() + + env := runningSession() + env.log = gateway.LogSummary{ + Decisions: 4, + Errors: 3, + LastError: "splice dial failed", + } + + check := egressCheck(t, Session(env, gatewayConfig())) + + assert.Equal(t, Warn, check.Status) + assert.Contains(t, check.Detail, "splice dial failed") + assert.Contains(t, check.Fix, "qubesome gateway logs") +} + +func TestSessionEgressWithAQuietGateway(t *testing.T) { + t.Parallel() + + env := runningSession() + env.log = gateway.LogSummary{Decisions: 12} + + check := egressCheck(t, Session(env, gatewayConfig())) + + assert.Equal(t, OK, check.Status) + assert.Contains(t, check.Detail, "12") +} + +// A gateway from before qubesome kept its log has none to read. That is +// not a fault of the session, and it must not be reported as one. +func TestSessionEgressWithoutALog(t *testing.T) { + t.Parallel() + + env := runningSession() + env.logErr = errors.New("no gateway log at /run/session/gateway.log") + + check := egressCheck(t, Session(env, gatewayConfig())) + + assert.Equal(t, Warn, check.Status) + assert.Contains(t, check.Detail, "no gateway log") +} + +// A session with no gateway running has no egress to diagnose, and +// stacking a second answer on the same cause buries the one worth reading. +func TestSessionEgressIsNotAskedWithoutAGateway(t *testing.T) { + t.Parallel() + + env := &fakeEnv{alive: map[string]bool{files.SessionStatePath(): true}} + + for _, c := range Session(env, gatewayConfig()) { + assert.NotEqual(t, "gateway egress", c.Name, + "there is no gateway, so there is nothing to say about its egress") + } +} + +func egressCheck(t *testing.T, checks []Check) Check { + t.Helper() + + for _, c := range checks { + if c.Name == "gateway egress" { + return c + } + } + t.Fatalf("no gateway egress check in %v", checks) + + return Check{} +} diff --git a/internal/files/files.go b/internal/files/files.go index 6dba4b1..5c26134 100644 --- a/internal/files/files.go +++ b/internal/files/files.go @@ -245,6 +245,18 @@ func GatewayStatePath() string { return filepath.Join(SessionDir(), "sandbox-gateway.json") } +// GatewayLogPath returns where the session's gateway writes what it says. +// +// The gateway is started by whichever launch found none running, and it is +// put in a session of its own so that a Ctrl-C at that terminal does not +// take the session's egress with it. Its stdout is therefore a terminal +// nobody is necessarily still watching, and the audit line behind a denied +// connection is the one thing a user needs when a workload cannot reach +// something. It goes here instead, and qubesome gateway logs reads it back. +func GatewayLogPath() string { + return filepath.Join(SessionDir(), "gateway.log") +} + // GatewaySocketDir returns the host directory the gateway's control socket // is created in. // diff --git a/internal/gateway/attach.go b/internal/gateway/attach.go index 24e9fd2..a6b898e 100644 --- a/internal/gateway/attach.go +++ b/internal/gateway/attach.go @@ -3,7 +3,9 @@ package gateway import ( "context" "log/slog" + "net" "net/netip" + "strconv" "github.com/qubesome/cli/internal/types" ) @@ -67,6 +69,25 @@ func Attached(cfg *types.Config, network string) (*Attach, error) { return &Attach{gateway: g, config: *cfg.Gateway, Addr: addr}, nil } +// ProxyAddr returns the endpoint a workload asks for a tunnel on. +// +// The gateway's own address is already the workload's default route and +// its resolver, so a workload could find it for itself. The port it could +// not, so what it is told is the pair, ready to be used as it is. +func (a *Attach) ProxyAddr() (string, error) { + subnet, err := a.config.SubnetPrefix() + if err != nil { + return "", err + } + + addr, err := GatewayAddr(subnet) + if err != nil { + return "", err + } + + return net.JoinHostPort(addr.String(), strconv.Itoa(inProxyPort)), nil +} + // Wire gives the sandbox at pid a link to the gateway, addressed at both // ends and with a resolver pointed at it. // diff --git a/internal/gateway/logline.go b/internal/gateway/logline.go new file mode 100644 index 0000000..29284a1 --- /dev/null +++ b/internal/gateway/logline.go @@ -0,0 +1,125 @@ +package gateway + +import "strings" + +// The gateway writes its log with slog's text handler, so a line is a +// sequence of key=value pairs and a value holding a space is quoted. This +// reads that, tolerantly: a line it cannot make sense of yields nothing +// rather than an error, because the gateway's log format is the gateway's +// to change and a reader of it should degrade to showing the line as it +// is rather than refusing to show anything. +// +// The keys qubesome reads are the ones the gateway's audit line carries. +const ( + // fieldWorkload is the name qubesome registered the workload under. + fieldWorkload = "workload" + + // fieldLevel is the slog level, which is how a line reporting a + // malfunction is told from one reporting a decision. + fieldLevel = "level" + + // fieldAction is the proxy's verdict: deny, splice or inject. + fieldAction = "action" + + // fieldHost is the host a decision was about. + fieldHost = "host" +) + +// logField returns the value of key in a log line, and "" when the line +// does not carry it. +func logField(line, key string) string { + // Anchored on a delimiter so that a key is not found inside another + // one: "load" must not match "workload=". + for i := 0; i+len(key)+1 <= len(line); i++ { + if i > 0 && line[i-1] != ' ' { + continue + } + if !strings.HasPrefix(line[i:], key+"=") { + continue + } + + return logValue(line[i+len(key)+1:]) + } + + return "" +} + +// logValue reads one value from the start of rest, which is either quoted +// or runs to the next space. +func logValue(rest string) string { + if strings.HasPrefix(rest, `"`) { + // A quoted value ends at the next quote that is not escaped. + // slog quotes a value holding a space, and escapes a quote + // within it. + var b strings.Builder + for i := 1; i < len(rest); i++ { + switch rest[i] { + case '\\': + if i+1 < len(rest) { + i++ + b.WriteByte(rest[i]) + } + case '"': + return b.String() + default: + b.WriteByte(rest[i]) + } + } + + return b.String() + } + + if i := strings.IndexByte(rest, ' '); i >= 0 { + return rest[:i] + } + + return rest +} + +// selector returns the test for whether a log line is about the workload +// being asked about. +// +// qubesome registers a workload with the gateway under its own name and +// its profile's, joined by a dash, and either half may hold a dash of its +// own. The pair therefore cannot be split back into two names with any +// certainty, so what is matched depends on how much was asked for: +// +// - both named: the registered name is exactly the two joined, which is +// the only unambiguous question of the three. +// - a profile alone: the registered name ends with it. +// - a workload alone: the registered name begins with it. +// +// A workload called "a-b" in profile "c" and one called "a" in profile +// "b-c" register the same name, and nothing here can tell them apart. +// Naming both halves is what avoids the question. +// +// A line naming no workload at all, which is what the gateway's own +// startup and shutdown lines look like, is not about the workload being +// asked about, so a filter drops it. Asking for no filter keeps +// everything. +func selector(profile, workload string) func(line string) bool { + if profile == "" && workload == "" { + return func(string) bool { return true } + } + + switch { + case profile != "" && workload != "": + want := workload + "-" + profile + + return func(line string) bool { return logField(line, fieldWorkload) == want } + + case profile != "": + suffix := "-" + profile + + return func(line string) bool { + return strings.HasSuffix(logField(line, fieldWorkload), suffix) + } + + default: + prefix := workload + "-" + + return func(line string) bool { + return strings.HasPrefix(logField(line, fieldWorkload), prefix) + } + } +} diff --git a/internal/gateway/logline_test.go b/internal/gateway/logline_test.go new file mode 100644 index 0000000..6908f45 --- /dev/null +++ b/internal/gateway/logline_test.go @@ -0,0 +1,87 @@ +package gateway + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +const decision = `time=2026-09-10T14:22:32.892Z level=INFO msg="proxy decision" ` + + `plane=proxy workload=cli-llm-work host=api.anthropic.com action=splice ` + + `reason="tunnelling TLS untouched"` + +func TestLogField(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + line string + key string + want string + }{ + {"a bare value", decision, "workload", "cli-llm-work"}, + {"the first field", decision, "time", "2026-09-10T14:22:32.892Z"}, + {"a quoted value", decision, "msg", "proxy decision"}, + {"the last field", decision, "reason", "tunnelling TLS untouched"}, + {"a key that is absent", decision, "profile", ""}, + {"a key that is only a suffix of another", decision, "load", ""}, + {"an empty line", "", "workload", ""}, + {"a value at the end with no newline", "level=INFO", "level", "INFO"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tc.want, logField(tc.line, tc.key)) + }) + } +} + +// The gateway knows a workload by the name qubesome registered, which is +// the workload's own name and its profile's joined by a dash. Both halves +// may hold a dash themselves, so the pair cannot be split back apart with +// any certainty. Naming both is therefore the exact question, and naming +// one alone is a prefix or a suffix of it. +func TestSelects(t *testing.T) { + t.Parallel() + + line := func(workload string) string { + return `level=INFO msg="proxy decision" workload=` + workload + ` action=splice` + } + + tests := []struct { + name string + profile string + workload string + line string + want bool + }{ + {"no filter takes everything", "", "", line("cli-llm-work"), true}, + {"no filter takes a line naming no workload", "", "", "msg=\"starting gateway\"", true}, + + {"both, matching", "work", "cli-llm", line("cli-llm-work"), true}, + {"both, wrong profile", "personal", "cli-llm", line("cli-llm-work"), false}, + {"both, wrong workload", "work", "chrome", line("cli-llm-work"), false}, + {"both, matching only as a prefix", "work", "cli", line("cli-llm-work"), false}, + + {"profile only", "work", "", line("cli-llm-work"), true}, + {"profile only, another profile", "personal", "", line("cli-llm-work"), false}, + {"profile only, the whole name", "cli-llm-work", "", line("cli-llm-work"), false}, + + {"workload only", "", "cli-llm", line("cli-llm-work"), true}, + {"workload only, another workload", "", "chrome", line("cli-llm-work"), false}, + {"workload only, the whole name", "", "cli-llm-work", line("cli-llm-work"), false}, + + {"a filtered line naming no workload is not about it", "work", "", "msg=\"starting gateway\"", false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + sel := selector(tc.profile, tc.workload) + assert.Equal(t, tc.want, sel(tc.line)) + }) + } +} diff --git a/internal/gateway/logs.go b/internal/gateway/logs.go new file mode 100644 index 0000000..4901191 --- /dev/null +++ b/internal/gateway/logs.go @@ -0,0 +1,266 @@ +package gateway + +import ( + "bufio" + "context" + "errors" + "fmt" + "io" + "log/slog" + "os" + "strings" + "time" + + "github.com/qubesome/cli/internal/files" +) + +// followInterval is how often a follow looks for more of the log. +// +// The gateway writes through a descriptor to a file and nothing notifies a +// reader of it, so this is a poll. It is short enough that a decision +// shows up while the workload that caused it is still on screen, and long +// enough that watching an idle gateway is not a busy loop. +const followInterval = 200 * time.Millisecond + +// maxLineLen bounds one log line, in the initial read and while +// following. A gateway writing without newlines cannot make a reader of +// its log hold the whole of it in memory. +const maxLineLen = 1 << 20 + +// LogOptions selects what ShowLogs prints. +type LogOptions struct { + // Path is the log to read. Empty means the session's own gateway log. + Path string + + // Profile and Workload narrow the log to the lines about one + // workload, one profile's workloads, or one workload wherever it + // runs. See selector for what each combination matches, and why + // naming both is the only exact question of the three. + Profile string + Workload string + + // Last is how many of the log's final matching lines to print. Zero + // prints all of them. + Last int + + // Follow keeps printing what is appended, until the context is done. + Follow bool +} + +func (o LogOptions) path() string { + if o.Path != "" { + return o.Path + } + + return files.GatewayLogPath() +} + +// ShowLogs writes the session gateway's log to w. +// +// The gateway is started by whichever qubesome run found none already +// running, and it is put in a session of its own so that a Ctrl-C at that +// terminal does not take the session's egress away with it. Its output +// therefore has nowhere to go that anybody could still be looking at, +// which is why it is written to a file and read back here. +func ShowLogs(ctx context.Context, w io.Writer, opts LogOptions) error { + path := opts.path() + + f, err := os.Open(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("there is no gateway log at %s: this session has not started a gateway", path) + } + + return fmt.Errorf("failed to open the gateway log %q: %w", path, err) + } + defer f.Close() + + selects := selector(opts.Profile, opts.Workload) + + if err := writeTail(w, f, opts.Last, selects); err != nil { + return err + } + + if !opts.Follow { + return nil + } + + return follow(ctx, w, f, selects) +} + +// writeTail writes the lines of f that selects accepts: all of them, or +// only the final few when last is set. +// +// The file is left positioned at its end either way, which is where a +// follow carries on from: the scanner stops having consumed everything up +// to EOF. +func writeTail(w io.Writer, f *os.File, last int, selects func(string) bool) error { + // A ring of the last lines wanted, so a log far larger than the + // answer is not held in memory to produce it. Zero means every line + // is written as it is read and nothing is held at all. + var ring []string + if last > 0 { + ring = make([]string, 0, last) + } + + scanner := bufio.NewScanner(f) + scanner.Buffer(make([]byte, 0, bufio.MaxScanTokenSize), maxLineLen) + + for scanner.Scan() { + line := scanner.Text() + if !selects(line) { + continue + } + + if last <= 0 { + if err := writeLine(w, line); err != nil { + return err + } + + continue + } + + if len(ring) == last { + ring = append(ring[:0], ring[1:]...) + } + ring = append(ring, line) + } + if err := scanner.Err(); err != nil { + return fmt.Errorf("failed to read the gateway log: %w", err) + } + + for _, line := range ring { + if err := writeLine(w, line); err != nil { + return err + } + } + + return nil +} + +func writeLine(w io.Writer, line string) error { + if _, err := io.WriteString(w, line+"\n"); err != nil { + return fmt.Errorf("failed to write the gateway log: %w", err) + } + + return nil +} + +// follow writes the lines appended to f that selects accepts, until ctx is +// done. +// +// Only whole lines are written. A line is what carries the workload a +// decision was about, so half of one cannot be matched against a filter, +// and printing it unmatched would show another workload's log to someone +// who asked not to see it. +func follow(ctx context.Context, w io.Writer, f *os.File, selects func(string) bool) error { + ticker := time.NewTicker(followInterval) + defer ticker.Stop() + + reader := bufio.NewReader(f) + var pending strings.Builder + + for { + select { + case <-ctx.Done(): + // The only way a follow ends is by being asked to stop, so + // that is not an error to report. + return nil + case <-ticker.C: + if err := followOnce(w, reader, &pending, selects); err != nil { + return err + } + } + } +} + +// followOnce drains what the reader can give without blocking, writing +// every whole line it completes. What is left over is kept in pending for +// the next tick, which is how a line still being written is not printed +// halfway. +func followOnce(w io.Writer, reader *bufio.Reader, pending *strings.Builder, selects func(string) bool) error { + for { + chunk, err := reader.ReadString('\n') + + // A read that stopped short of a newline is a line the gateway + // has not finished writing. It is held until it has. + if errors.Is(err, io.EOF) { + if pending.Len()+len(chunk) > maxLineLen { + pending.Reset() + + return nil + } + pending.WriteString(chunk) + + return nil + } + if err != nil { + return fmt.Errorf("failed to read the gateway log: %w", err) + } + + line := strings.TrimSuffix(chunk, "\n") + if pending.Len() > 0 { + line = pending.String() + line + pending.Reset() + } + + if !selects(line) { + continue + } + if err := writeLine(w, line); err != nil { + return err + } + } +} + +// appendLog opens the gateway's log to add to what is already there. +// +// It is what the uplink writes through. The uplink is started once the +// gateway sandbox is up, so the log it joins is the one that sandbox's +// launch has already begun, and truncating here would throw away the lines +// explaining a gateway that had trouble coming up. +func appendLog(path string) (*os.File, error) { + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_APPEND, files.FileMode) + if err != nil { + return nil, fmt.Errorf("failed to open the gateway log %q: %w", path, err) + } + + return f, nil +} + +// openLog opens the gateway's log for the sandbox to write to. +// +// It is truncated rather than appended to. The log is the record of the +// gateway that is running, and one gateway replaces another only by the +// first having gone, so carrying the old one's lines forward would only +// make it harder to tell which of them explains what is happening now. +// +// 0600 because a gateway log holds every name a workload asked for and +// every host it was allowed or refused, which is a record of what the user +// was doing. +func openLog(path string) (*os.File, error) { + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, files.FileMode) + if err != nil { + return nil, fmt.Errorf("failed to open the gateway log %q: %w", path, err) + } + + return f, nil +} + +// closeLog closes qubesome's own copy of the gateway log. +// +// The sandbox and the uplink are handed their own descriptors for it, so +// this one is closed as soon as they are started and closing it loses +// nothing: nothing here writes through it, so there is no buffered tail +// of a write to fail to reach the disk. +// +// It is still not discarded. A close that fails says the filesystem the +// log sits on is unwell, and that is worth knowing about the file a +// gateway explains itself in. It is a warning and not an error because +// the gateway it belongs to is already running by this point, and a log +// qubesome could not close is no reason to take one down. +func closeLog(f *os.File) { + if err := f.Close(); err != nil { + slog.Warn("failed to close the gateway log", "path", f.Name(), "error", err) + } +} diff --git a/internal/gateway/logs_test.go b/internal/gateway/logs_test.go new file mode 100644 index 0000000..a83db07 --- /dev/null +++ b/internal/gateway/logs_test.go @@ -0,0 +1,337 @@ +package gateway + +import ( + "bytes" + "context" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeLog(t *testing.T, lines ...string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), "gateway.log") + var body []byte + for _, l := range lines { + body = append(body, l...) + body = append(body, '\n') + } + require.NoError(t, os.WriteFile(path, body, 0o600)) + + return path +} + +func TestShowLogs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + lines []string + last int + want string + }{ + { + name: "the whole log", + lines: []string{"one", "two", "three"}, + want: "one\ntwo\nthree\n", + }, + { + name: "the last lines", + lines: []string{"one", "two", "three"}, + last: 2, + want: "two\nthree\n", + }, + { + name: "more lines than the log holds", + lines: []string{"one"}, + last: 10, + want: "one\n", + }, + { + name: "an empty log", + lines: nil, + want: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + var out bytes.Buffer + require.NoError(t, ShowLogs(t.Context(), &out, LogOptions{ + Path: writeLog(t, tc.lines...), + Last: tc.last, + })) + assert.Equal(t, tc.want, out.String()) + }) + } +} + +// A session with no gateway has no log, and saying so is not a failure of +// the command. It is the answer to what was asked. +func TestShowLogsWithoutALog(t *testing.T) { + t.Parallel() + + var out bytes.Buffer + err := ShowLogs(t.Context(), &out, LogOptions{ + Path: filepath.Join(t.TempDir(), "absent.log"), + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), "no gateway log") + assert.Empty(t, out.String()) +} + +// syncBuffer is a bytes.Buffer the follower writes to while the test reads. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + + return b.buf.String() +} + +func TestShowLogsFollows(t *testing.T) { + t.Parallel() + + path := writeLog(t, "first") + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + out := &syncBuffer{} + done := make(chan error, 1) + go func() { + done <- ShowLogs(ctx, out, LogOptions{Path: path, Follow: true}) + }() + + require.EventuallyWithT(t, func(c *assert.CollectT) { + assert.Equal(c, "first\n", out.String()) + }, time.Second, 10*time.Millisecond, "what the log already held must be printed") + + f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0o600) + require.NoError(t, err) + _, err = f.WriteString("second\n") + require.NoError(t, err) + require.NoError(t, f.Close()) + + require.EventuallyWithT(t, func(c *assert.CollectT) { + assert.Equal(c, "first\nsecond\n", out.String()) + }, time.Second, 10*time.Millisecond, "a line appended while following must be printed") + + cancel() + require.NoError(t, <-done, "a cancelled follow is how the command ends, not a failure") +} + +// The log describes the gateway that is running, so starting one begins the +// log afresh rather than adding to what the last one said. +func TestOpenLogTruncates(t *testing.T) { + t.Parallel() + + path := writeLog(t, "what the last gateway said") + + f, err := openLog(path) + require.NoError(t, err) + defer f.Close() + + _, err = f.WriteString("what this one says\n") + require.NoError(t, err) + + body, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, "what this one says\n", string(body)) +} + +func TestOpenLogIsPrivate(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "gateway.log") + + f, err := openLog(path) + require.NoError(t, err) + require.NoError(t, f.Close()) + + st, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o600), st.Mode().Perm(), + "the log carries the names a workload asked for, so it is the user's alone") +} + +// The uplink joins the log the gateway sandbox's launch already began, +// rather than starting it over and dropping what explains a gateway that +// had trouble coming up. +func TestAppendLogKeepsWhatIsThere(t *testing.T) { + t.Parallel() + + path := writeLog(t, "the gateway is starting") + + f, err := appendLog(path) + require.NoError(t, err) + defer f.Close() + + _, err = f.WriteString("the uplink is up\n") + require.NoError(t, err) + + body, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, "the gateway is starting\nthe uplink is up\n", string(body)) +} + +func TestShowLogsFilters(t *testing.T) { + t.Parallel() + + lines := []string{ + `msg="starting gateway"`, + `msg="proxy decision" workload=chrome-work host=a.example action=splice`, + `msg="proxy decision" workload=cli-llm-work host=b.example action=deny`, + `msg="proxy decision" workload=cli-llm-personal host=c.example action=splice`, + `msg="proxy decision" workload=chrome-personal host=d.example action=splice`, + } + + tests := []struct { + name string + profile string + workload string + want []string + }{ + { + name: "no filter shows the whole log", + want: lines, + }, + { + name: "a profile shows every workload in it", + profile: "work", + want: []string{lines[1], lines[2]}, + }, + { + name: "a workload shows it in every profile", + workload: "cli-llm", + want: []string{lines[2], lines[3]}, + }, + { + name: "both name one workload exactly", + profile: "work", + workload: "cli-llm", + want: []string{lines[2]}, + }, + { + name: "a pair that ran nothing shows nothing", + profile: "work", + workload: "obsidian", + want: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + var out bytes.Buffer + require.NoError(t, ShowLogs(t.Context(), &out, LogOptions{ + Path: writeLog(t, lines...), + Profile: tc.profile, + Workload: tc.workload, + })) + + var want string + for _, l := range tc.want { + want += l + "\n" + } + assert.Equal(t, want, out.String()) + }) + } +} + +// The last lines of what was asked for, not the last lines of the file +// with the filter applied afterwards. Otherwise asking for one workload's +// last twenty lines shows however many of its lines happen to fall in the +// file's last twenty. +func TestShowLogsCountsTheLinesItShows(t *testing.T) { + t.Parallel() + + lines := []string{ + `workload=cli-llm-work host=first action=splice`, + `workload=chrome-work host=noise action=splice`, + `workload=chrome-work host=noise action=splice`, + `workload=chrome-work host=noise action=splice`, + `workload=cli-llm-work host=last action=splice`, + } + + var out bytes.Buffer + require.NoError(t, ShowLogs(t.Context(), &out, LogOptions{ + Path: writeLog(t, lines...), + Workload: "cli-llm", + Last: 2, + })) + + assert.Equal(t, lines[0]+"\n"+lines[4]+"\n", out.String()) +} + +func TestShowLogsFollowsWithAFilter(t *testing.T) { + t.Parallel() + + path := writeLog(t, `workload=cli-llm-work host=first action=splice`) + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + out := &syncBuffer{} + done := make(chan error, 1) + go func() { + done <- ShowLogs(ctx, out, LogOptions{Path: path, Workload: "cli-llm", Follow: true}) + }() + + require.EventuallyWithT(t, func(c *assert.CollectT) { + assert.Contains(c, out.String(), "host=first") + }, time.Second, 10*time.Millisecond) + + f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0o600) + require.NoError(t, err) + _, err = f.WriteString("workload=chrome-work host=ignored action=splice\n" + + "workload=cli-llm-work host=second action=splice\n") + require.NoError(t, err) + require.NoError(t, f.Close()) + + require.EventuallyWithT(t, func(c *assert.CollectT) { + assert.Contains(c, out.String(), "host=second") + }, time.Second, 10*time.Millisecond) + + assert.NotContains(t, out.String(), "host=ignored", + "a followed line for another workload must not be printed") + + cancel() + require.NoError(t, <-done) +} + +// A close that fails is reported and does not stop the caller. The +// gateway it belongs to is already running by then, and a log qubesome +// could not close is no reason to take one down. +func TestCloseLogSurvivesAFailure(t *testing.T) { + t.Parallel() + + f, err := openLog(filepath.Join(t.TempDir(), "gateway.log")) + require.NoError(t, err) + + require.NoError(t, f.Close()) + + // The second close is the failure, since the descriptor is gone. + assert.NotPanics(t, func() { closeLog(f) }) +} diff --git a/internal/gateway/proxyaddr_test.go b/internal/gateway/proxyaddr_test.go new file mode 100644 index 0000000..2aaa2ef --- /dev/null +++ b/internal/gateway/proxyaddr_test.go @@ -0,0 +1,48 @@ +package gateway + +import ( + "testing" + + "github.com/qubesome/cli/internal/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A workload reaches the proxy at the gateway's own address, which is the +// first host address of the subnet and is already its default route and +// its resolver. What it cannot work out for itself is the port, so the +// whole endpoint is what it is told. +func TestAttachProxyAddr(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + subnet string + want string + }{ + {"the usual subnet", "10.111.0.0/24", "10.111.0.1:3128"}, + {"another range", "192.168.44.0/24", "192.168.44.1:3128"}, + {"a small one", "10.9.9.8/30", "10.9.9.9:3128"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + a := &Attach{config: types.GatewayConfig{Subnet: tc.subnet}} + + got, err := a.ProxyAddr() + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestAttachProxyAddrWithoutASubnet(t *testing.T) { + t.Parallel() + + a := &Attach{config: types.GatewayConfig{Subnet: "not a subnet"}} + + _, err := a.ProxyAddr() + require.Error(t, err) +} diff --git a/internal/gateway/run.go b/internal/gateway/run.go index 1c626a0..d996f16 100644 --- a/internal/gateway/run.go +++ b/internal/gateway/run.go @@ -56,6 +56,17 @@ const ( // for the whole session, so unlike a workload's it carries no profile. gatewayHostname = "qubesome-gateway" + // inProxyPort is the port the gateway image's proxy serves its + // plaintext listener on, which is also where it accepts CONNECT. + // + // It is here with the image's other constants, and carries the same + // caveat: it belongs to the gateway and not to qubesome, so the two + // have to be changed together. A workload is told the whole endpoint + // rather than only the address for exactly that reason, so that a + // port which is the gateway's business does not end up written into + // anybody's dotfiles. + inProxyPort = 3128 + // pastaCommand is the uplink binary in the gateway image, where the // passt package puts it. pastaCommand = "/usr/bin/pasta" @@ -394,8 +405,27 @@ func (g Gateway) launch(bundle images.Bundle, spec sandbox.Spec) error { // session whose gateway ended at the first Ctrl-C would take the egress // of every workload still running with it. cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + + // Not this process's stdout. The gateway outlives the launch, so what + // it says would go to a terminal that is not necessarily still there, + // interleaved with the output of the workload that happened to start + // it. qubesome gateway logs reads this back. + log, err := openLog(files.GatewayLogPath()) + if err != nil { + return err + } + // The sandbox has its own copy once it is started, and a launch that + // never got that far has nothing to write here either. + // + // The error is reported rather than deferred away. Nothing here ever + // writes through this descriptor, so unlike the write in replace + // there is no last part of one that reaches the filesystem at close + // and nothing to lose. What a failure here does say is that the + // filesystem holding the log is unwell, and the log is where a + // gateway that goes wrong explains itself, so it is worth a line. + defer closeLog(log) + cmd.Stdout = log + cmd.Stderr = log err = cmd.Start() @@ -691,8 +721,17 @@ func (h helper) start() (*execabs.Cmd, error) { // Ctrl-C at the terminal that started a workload is not a request to // take it away. cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + + // The gateway's log, which the sandbox's launch has already begun by + // the time an uplink is put in its namespace. It goes there for the + // reason the gateway's own output does: it outlives the launch. + log, err := appendLog(files.GatewayLogPath()) + if err != nil { + return nil, err + } + defer closeLog(log) + cmd.Stdout = log + cmd.Stderr = log if err := cmd.Start(); err != nil { return nil, err @@ -1088,10 +1127,17 @@ func (g Gateway) readAlloc(subnet netip.Prefix) (allocation, error) { return allocation{}, fmt.Errorf("failed to parse the gateway addresses %q: %w", g.AllocPath, err) } + // The opening clause is the one the status message uses, so the two + // describe the same thing in the same words. This one keeps the claim + // about a running gateway that the status message drops: Allocate is + // only reached after Up, so by here there is one, and it is the + // reason the remedy is a restart rather than an edit. A status is + // read in the state gateway stop leaves, where there is not. if a.Subnet != subnet.String() { return allocation{}, fmt.Errorf( - "this session's gateway hands addresses out of %s and the config now asks for %s: "+ - "the running gateway holds the first address of the old range, so the session has to be restarted", + "this session has handed addresses out of %s and the config now asks for %s: "+ + "the gateway running in it holds the first address of the old range, "+ + "so the session has to be restarted before the new range is used", a.Subnet, subnet) } diff --git a/internal/gateway/run_test.go b/internal/gateway/run_test.go index c9cc0f6..7a3fbcc 100644 --- a/internal/gateway/run_test.go +++ b/internal/gateway/run_test.go @@ -467,3 +467,28 @@ func prefix(t *testing.T, s string) netip.Prefix { return p } + +// A subnet changed under a session that has already handed addresses out +// is refused, because the count belongs to the old range and a gateway in +// the session holds its first address. +// +// The message opens the way the status one does, so the two describe the +// same thing in the same words. It keeps the claim about a running +// gateway that the status message drops: Allocate is only ever reached +// after Up, so by here there is one, and it is why a restart is the +// remedy rather than an edit. +func TestAllocateRefusesAChangedSubnet(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + _, err := g.Allocate(prefix(t, testSubnet)) + require.NoError(t, err) + + _, err = g.Allocate(prefix(t, "10.112.0.0/24")) + + require.Error(t, err) + assert.Contains(t, err.Error(), "this session has handed addresses out of 10.111.0.0/24") + assert.Contains(t, err.Error(), "the config now asks for 10.112.0.0/24") + assert.Contains(t, err.Error(), "the session has to be restarted") +} diff --git a/internal/gateway/status.go b/internal/gateway/status.go new file mode 100644 index 0000000..e8a76e9 --- /dev/null +++ b/internal/gateway/status.go @@ -0,0 +1,332 @@ +package gateway + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "time" + + "github.com/qubesome/cli/internal/sandbox" + "github.com/qubesome/cli/internal/session" + "github.com/qubesome/cli/internal/types" +) + +// statusReadyTimeout bounds the readiness question a status asks. +// +// It is not the client's own ten minutes. That deadline is sized for a +// launch, where Ready may be waiting on an image that is still being +// unpacked and where waiting is the whole job. A status is there to report, +// so a gateway that does not answer in a few seconds has to become a line in +// the report rather than a command that does not return. doctor bounds the +// same call for the same reason. +const statusReadyTimeout = 5 * time.Second + +// Status is what qubesome knows about the session's gateway. +// +// Every field comes from something qubesome already holds: the config that +// names a gateway, the records a launch writes, and the readiness call the +// gateway has always served. Nothing here asks the gateway anything new, so +// a status needs no newer gateway than the one that is running. +type Status struct { + // ConfigProblem says why Image, Policy and Subnet are empty. It is + // empty when they are not. A status that cannot find a config says so, + // rather than printing three blank lines that read as a gateway + // configured with nothing. + ConfigProblem string + + Image string + Policy string + Subnet string + + // PolicyProblem says why the policy file path could not be resolved + // against the directory the config was read from. + PolicyProblem string + + // HolderPath and StatePath are the records that were read. They are + // reported because they are the files an operator would otherwise have + // to go looking for. + HolderPath string + StatePath string + + HolderRunning bool + HolderPID int + + Running bool + PID int + + // ReadyErr is why a running gateway did not report itself ready. It is + // empty both when it did and when there was no gateway to ask. + ReadyErr string + + // GatewayAddr is the address the gateway holds on every veth. Allocated + // is how many workload addresses have been handed out since it started, + // and LastAddr is the highest of them. + GatewayAddr string + Allocated uint64 + LastAddr string + + // AddrProblem says why the addresses are not in the report. + AddrProblem string +} + +// Inspect gathers what is known about the session's gateway. +// +// cfg is the whole qubesome config rather than its gateway block, because a +// config that did not load and a config with no gateway block are different +// answers and a nil block cannot tell them apart. A bare status reaches a +// config only through a running profile or the user-level file, so it may +// well have neither, and reporting "no gateway is configured" for "no config +// was read" is how doctor once said there was no gateway on a host whose +// config configures one. +// +// ready is called only when a gateway is recorded as running. A readiness +// failure whose whole cause is that nothing is listening repeats the line +// above it. +func (g Gateway) Inspect(s session.Session, cfg *types.Config, ready func() error) Status { + st := Status{ + HolderPath: s.StatePath, + StatePath: g.StatePath, + } + + // sandbox.Alive and not a look at the pid. It compares the recorded + // start time too, so a record left behind by a gateway that crashed + // reads as not running rather than as whatever process has since been + // given its number. + st.HolderRunning = sandbox.Alive(s.StatePath) + if st.HolderRunning { + if rec, err := sandbox.ReadState(s.StatePath); err == nil { + st.HolderPID = rec.PID + } + } + + st.Running = sandbox.Alive(g.StatePath) + if st.Running { + if rec, err := sandbox.ReadState(g.StatePath); err == nil { + st.PID = rec.PID + } + } + + g.inspectConfig(&st, cfg) + + if st.Running && ready != nil { + if err := ready(); err != nil { + st.ReadyErr = err.Error() + } + } + + return st +} + +// inspectConfig fills in what the config says a gateway should be, and the +// addresses handed out of the subnet it names. +func (g Gateway) inspectConfig(st *Status, cfg *types.Config) { + if cfg == nil { + st.ConfigProblem = "no qubesome config was loaded, so the image, policy and subnet it names are unknown" + return + } + + if cfg.Gateway == nil { + st.ConfigProblem = "no gateway is configured, so workloads run with no egress" + return + } + + st.Image = cfg.Gateway.Image + st.Subnet = cfg.Gateway.Subnet + + policy, err := cfg.Gateway.ConfigPath(cfg.RootDir) + if err != nil { + st.PolicyProblem = err.Error() + } else { + st.Policy = policy + } + + g.inspectAddrs(st, cfg.Gateway) +} + +// inspectAddrs reports the gateway's own address and how much of the subnet +// has been handed out. +// +// The count is read straight from the record rather than through readAlloc, +// which refuses a record naming a different subnet from the one asked for. +// That refusal is right for a launch, which must not hand out an address the +// running gateway knows nothing about. A status has to be able to report the +// mismatch instead of failing on it, since a config edited under a running +// gateway is one of the things it exists to show. +func (g Gateway) inspectAddrs(st *Status, cfg *types.GatewayConfig) { + subnet, err := cfg.SubnetPrefix() + if err != nil { + st.AddrProblem = err.Error() + return + } + + addr, err := GatewayAddr(subnet) + if err != nil { + st.AddrProblem = err.Error() + return + } + st.GatewayAddr = addr.String() + + a, err := readAllocFile(g.AllocPath) + if err != nil { + st.AddrProblem = err.Error() + return + } + + // What this describes is the record, not a gateway. The record + // outlives the gateway that wrote it, and gateway stop leaves exactly + // that behind: nothing running, and a note of the range the last one + // handed addresses out of. A status naming a running gateway would be + // false in the state it is most likely to be read in. + if a.Subnet != "" && a.Subnet != subnet.String() { + st.AddrProblem = fmt.Sprintf( + "this session has handed addresses out of %s and the config now asks for %s, "+ + "so the session has to be restarted before the new range is used", + a.Subnet, subnet) + return + } + + st.Allocated = a.Allocated + if a.Allocated == 0 { + return + } + + // The first workload takes the address above the gateway's, and the + // count only ever goes up, so the highest handed out is the one that + // many places past it. + last, err := addrAt(subnet, a.Allocated+1) + if err != nil { + st.AddrProblem = err.Error() + return + } + st.LastAddr = last.String() +} + +// readAllocFile returns the address record, or an empty one when a gateway +// has handed nothing out yet. +func readAllocFile(path string) (allocation, error) { + data, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return allocation{}, nil + } + if err != nil { + return allocation{}, fmt.Errorf("failed to read the gateway addresses %q: %w", path, err) + } + + var a allocation + if err := json.Unmarshal(data, &a); err != nil { + return allocation{}, fmt.Errorf("failed to parse the gateway addresses %q: %w", path, err) + } + + return a, nil +} + +// StatusReady asks the running gateway whether its resolver, proxy and +// netfilter ruleset are up, under a deadline a status command can wait out. +func (g Gateway) StatusReady() error { + ctx, cancel := context.WithTimeout(context.Background(), statusReadyTimeout) + defer cancel() + + c, err := g.Client() + if err != nil { + return err + } + + return c.Ready(ctx) +} + +// statusLine is one labelled fact. +type statusLine struct { + label string + text string +} + +// Write renders the status, one labelled line per fact. +func (s Status) Write(w io.Writer) error { + for _, l := range s.lines() { + if _, err := fmt.Fprintf(w, "%-10s %s\n", l.label, l.text); err != nil { + return err + } + } + + return nil +} + +// lines is the report in the order it is read in: what a gateway was asked +// to be, then whether there is one, then what it is doing. +func (s Status) lines() []statusLine { + out := make([]statusLine, 0, 7) + + if s.ConfigProblem != "" { + out = append(out, statusLine{"config", s.ConfigProblem}) + } else { + policy := s.Policy + if s.PolicyProblem != "" { + policy = "cannot be resolved: " + s.PolicyProblem + } + + out = append(out, + statusLine{"image", s.Image}, + statusLine{"policy", policy}, + statusLine{"subnet", s.Subnet}, + ) + } + + out = append(out, + statusLine{"holder", alive(s.HolderRunning, s.HolderPID, s.HolderPath)}, + statusLine{"gateway", alive(s.Running, s.PID, s.StatePath)}, + ) + + if s.Running { + out = append(out, statusLine{"readiness", s.readiness()}) + } + + if line, ok := s.addresses(); ok { + out = append(out, statusLine{"addresses", line}) + } + + return out +} + +// alive words the liveness of one recorded process. The record is named in +// both cases, because it is the file that answers the question and the one +// an operator reaches for next. +func alive(running bool, pid int, path string) string { + if !running { + return fmt.Sprintf("not running (no live record in %s)", path) + } + + return fmt.Sprintf("running, pid %d (%s)", pid, path) +} + +func (s Status) readiness() string { + if s.ReadyErr != "" { + return "not ready: " + s.ReadyErr + } + + return "the resolver, proxy and ruleset are up" +} + +// addresses words what the subnet has been used for, and reports whether +// there is anything to say. There is nothing without a subnet to say it of. +func (s Status) addresses() (string, bool) { + if s.AddrProblem != "" { + return s.AddrProblem, true + } + + if s.GatewayAddr == "" { + return "", false + } + + // The count belongs to the gateway that is running. A record left by an + // earlier one is removed by the next launch, so reporting it against no + // gateway would name addresses nothing holds. + if !s.Running || s.Allocated == 0 { + return fmt.Sprintf("%s is the gateway's own", s.GatewayAddr), true + } + + return fmt.Sprintf("%s is the gateway's own, %d handed out up to %s", + s.GatewayAddr, s.Allocated, s.LastAddr), true +} diff --git a/internal/gateway/status_test.go b/internal/gateway/status_test.go new file mode 100644 index 0000000..39b753a --- /dev/null +++ b/internal/gateway/status_test.go @@ -0,0 +1,278 @@ +// A real gateway cannot be started here, for the reasons run_test.go gives, +// so what a status says about one that is genuinely up is not covered. What +// is covered is everything the report is built from: the config it reads, +// the records it reads them beside, a record left behind by a gateway that +// crashed, the addresses handed out of the subnet, and when the gateway is +// asked whether it is ready at all. +package gateway + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/qubesome/cli/internal/sandbox" + "github.com/qubesome/cli/internal/session" + "github.com/qubesome/cli/internal/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestStatusWithoutAConfig(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + st := g.Inspect(newTestSession(t), nil, failingReady(t)) + + assert.Contains(t, st.ConfigProblem, "no qubesome config was loaded") + assert.Empty(t, st.Image) + assert.Empty(t, st.Subnet) + + out := render(t, st) + assert.Contains(t, out, "config no qubesome config was loaded") + assert.NotContains(t, out, "image ") + assert.NotContains(t, out, "subnet ") + assert.NotContains(t, out, "addresses ") +} + +// A config with no gateway block is a supported configuration and not a +// config that failed to load. Saying the first for the second is the bug +// doctor's session checks were fixed for. +func TestStatusWithoutAGatewayBlock(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + st := g.Inspect(newTestSession(t), &types.Config{}, failingReady(t)) + + assert.Contains(t, st.ConfigProblem, "no gateway is configured") + assert.NotContains(t, st.ConfigProblem, "no qubesome config was loaded") +} + +func TestStatusReportsTheConfiguredGateway(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + st := g.Inspect(newTestSession(t), testConfig(unusableConfig()), failingReady(t)) + + require.Empty(t, st.ConfigProblem) + assert.Equal(t, "ghcr.io/qubesome/gateway:latest", st.Image) + assert.Equal(t, testSubnet, st.Subnet) + + // A path in a qubesome config is rooted at the config tree, so the + // leading separator names that tree and not the root of the disk. + assert.Equal(t, "/config/root/gateway.yml", st.Policy) + assert.Empty(t, st.PolicyProblem) + + out := render(t, st) + assert.Contains(t, out, "image ghcr.io/qubesome/gateway:latest") + assert.Contains(t, out, "policy /config/root/gateway.yml") + assert.Contains(t, out, "subnet "+testSubnet) +} + +func TestStatusReportsAPolicyPathThatLeavesTheConfigTree(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + cfg := unusableConfig() + cfg.Config = "../gateway.yml" + + st := g.Inspect(newTestSession(t), testConfig(cfg), failingReady(t)) + + assert.Empty(t, st.Policy) + assert.NotEmpty(t, st.PolicyProblem) + assert.Contains(t, render(t, st), "policy cannot be resolved:") +} + +func TestStatusReportsNothingRunning(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + s := newTestSession(t) + + st := g.Inspect(s, testConfig(unusableConfig()), failingReady(t)) + + assert.False(t, st.HolderRunning) + assert.False(t, st.Running) + + out := render(t, st) + assert.Contains(t, out, "holder not running (no live record in "+s.StatePath+")") + assert.Contains(t, out, "gateway not running (no live record in "+g.StatePath+")") + + // Readiness only says something once there is a gateway to ask. + assert.NotContains(t, out, "readiness") +} + +// A record outlives the process it names, and the pid in it may since have +// been given to something else. Alive compares the recorded start time, +// which is what makes such a record read as no gateway at all. +func TestStatusReadsAStaleRecordAsNotRunning(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + state := fmt.Sprintf(`{"pid":%d,"startTime":1}`, os.Getpid()) + require.NoError(t, os.WriteFile(g.StatePath, []byte(state), 0o600)) + + st := g.Inspect(newTestSession(t), testConfig(unusableConfig()), failingReady(t)) + + assert.False(t, st.Running) + assert.Zero(t, st.PID) + assert.Empty(t, st.ReadyErr) + assert.Contains(t, render(t, st), "gateway not running") +} + +func TestStatusReportsARunningHolderAndGateway(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + s := newTestSession(t) + + require.NoError(t, sandbox.WriteState(s.StatePath, os.Getpid())) + require.NoError(t, sandbox.WriteState(g.StatePath, os.Getpid())) + + st := g.Inspect(s, testConfig(unusableConfig()), func() error { return nil }) + + assert.True(t, st.HolderRunning) + assert.Equal(t, os.Getpid(), st.HolderPID) + assert.True(t, st.Running) + assert.Equal(t, os.Getpid(), st.PID) + + out := render(t, st) + assert.Contains(t, out, fmt.Sprintf("holder running, pid %d", os.Getpid())) + assert.Contains(t, out, fmt.Sprintf("gateway running, pid %d", os.Getpid())) + assert.Contains(t, out, "readiness the resolver, proxy and ruleset are up") +} + +// A gateway that does not answer is a status worth reporting rather than +// something to wait out, so the failure becomes a line like any other. +func TestStatusReportsAGatewayThatIsNotReady(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + require.NoError(t, sandbox.WriteState(g.StatePath, os.Getpid())) + + st := g.Inspect(newTestSession(t), testConfig(unusableConfig()), + func() error { return errors.New("context deadline exceeded") }) + + assert.Equal(t, "context deadline exceeded", st.ReadyErr) + assert.Contains(t, render(t, st), "readiness not ready: context deadline exceeded") +} + +func TestStatusReportsTheAddressesHandedOut(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + require.NoError(t, sandbox.WriteState(g.StatePath, os.Getpid())) + + require.NoError(t, os.WriteFile(g.AllocPath, + []byte(fmt.Sprintf(`{"subnet":%q,"allocated":3}`, testSubnet)), 0o600)) + + st := g.Inspect(newTestSession(t), testConfig(unusableConfig()), func() error { return nil }) + + assert.Equal(t, "10.111.0.1", st.GatewayAddr) + assert.Equal(t, uint64(3), st.Allocated) + assert.Equal(t, "10.111.0.4", st.LastAddr) + assert.Contains(t, render(t, st), "addresses 10.111.0.1 is the gateway's own, 3 handed out up to 10.111.0.4") +} + +// The count belongs to the gateway that is running, and the next launch +// clears it, so it says nothing while there is no gateway. +func TestStatusReportsOnlyTheGatewayAddressWithNoGatewayRunning(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + require.NoError(t, os.WriteFile(g.AllocPath, + []byte(fmt.Sprintf(`{"subnet":%q,"allocated":3}`, testSubnet)), 0o600)) + + st := g.Inspect(newTestSession(t), testConfig(unusableConfig()), failingReady(t)) + + assert.Contains(t, render(t, st), "addresses 10.111.0.1 is the gateway's own\n") +} + +// A subnet changed under the record is one of the things a status is for, +// so it is reported rather than refused the way a launch refuses it. +// +// What is reported describes the record and not a gateway. The record +// outlives the gateway that wrote it, and gateway stop leaves exactly +// that behind: no gateway running and a record of the addresses the last +// one handed out. Saying a running gateway hands addresses out of +// anything is false in the state this is most likely to be read in, and +// this test is in it, since nothing is running here. +func TestStatusReportsASubnetTheRecordDoesNotMatch(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + require.NoError(t, os.WriteFile(g.AllocPath, []byte(`{"subnet":"10.112.0.0/24","allocated":1}`), 0o600)) + + st := g.Inspect(newTestSession(t), testConfig(unusableConfig()), failingReady(t)) + + require.False(t, st.Running, "the state this describes is one with no gateway in it") + assert.NotContains(t, st.AddrProblem, "running gateway", + "there is no running gateway to be handing anything out") + assert.Contains(t, st.AddrProblem, "10.112.0.0/24") + assert.Contains(t, render(t, st), "addresses this session has handed addresses out of 10.112.0.0/24") +} + +func TestStatusReportsAnUnusableSubnet(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + cfg := unusableConfig() + cfg.Subnet = "10.111.0.0" + + st := g.Inspect(newTestSession(t), testConfig(cfg), failingReady(t)) + + assert.Empty(t, st.GatewayAddr) + assert.Contains(t, st.AddrProblem, "invalid gateway subnet") +} + +// newTestSession returns a session whose files are in the test's own +// directory, so nothing here reads the user's session. +func newTestSession(t *testing.T) session.Session { + t.Helper() + + dir := t.TempDir() + + return session.Session{ + Dir: dir, + LockPath: filepath.Join(dir, "lock"), + StatePath: filepath.Join(dir, "holder.json"), + } +} + +// testRoot stands in for the directory a qubesome config was read from, +// which is what a policy path in it is resolved against. +const testRoot = "/config/root" + +func testConfig(gw types.GatewayConfig) *types.Config { + return &types.Config{RootDir: testRoot, Gateway: &gw} +} + +// failingReady is the readiness probe for a test that expects no gateway to +// be asked. +func failingReady(t *testing.T) func() error { + t.Helper() + + return func() error { + t.Error("the gateway was asked whether it is ready with none running") + return nil + } +} + +func render(t *testing.T, st Status) string { + t.Helper() + + var b strings.Builder + require.NoError(t, st.Write(&b)) + + return b.String() +} diff --git a/internal/gateway/stop.go b/internal/gateway/stop.go new file mode 100644 index 0000000..b12ea54 --- /dev/null +++ b/internal/gateway/stop.go @@ -0,0 +1,130 @@ +package gateway + +import ( + "errors" + "fmt" + "os" + "syscall" + "time" + + "github.com/qubesome/cli/internal/files" + "github.com/qubesome/cli/internal/sandbox" +) + +// Stop takes the session's gateway down and returns the pid it stopped, or +// zero when none was running. +// +// The session's namespace holder is left alone. A launch reuses whatever +// gateway it finds running, so a change to the gateway block reaches nothing +// until the running one is gone, and that is all this is for. Taking the +// holder down as well would make the next launch rebuild a user namespace +// that was never the problem, and every sandbox of the session nests inside +// that one. +// +// A session with no gateway in it is not an error to ask for. It is the +// state the caller wanted, and saying so is the answer rather than a +// failure. +func (g Gateway) Stop() (int, error) { + // The lock file lives in the session directory, which a host that has + // never started a session does not have. + if err := os.MkdirAll(g.Dir, files.DirMode); err != nil { + return 0, fmt.Errorf("failed to create the session dir %q: %w", g.Dir, err) + } + + // The same lock startOnce takes, and for the same reason. A launch that + // is between its own check and the record it writes would otherwise + // have its gateway killed and its record kept, leaving the session + // naming a process that is already gone. + lock, err := acquire(g.LockPath) + if err != nil { + return 0, err + } + defer lock.Close() + + // sandbox.Alive and not a look at the pid. It compares the recorded + // start time as well, so a record left behind by a gateway that crashed + // reads as not running, and nothing here signals a pid the kernel has + // since given to something else. + if !sandbox.Alive(g.StatePath) { + // Removing a record that already reads as not running changes no + // answer. It is removed because reap would have removed it had the + // launch that started the gateway still been there to do it. + if err := os.Remove(g.StatePath); err != nil && !errors.Is(err, os.ErrNotExist) { + return 0, fmt.Errorf("failed to remove the gateway state %q: %w", g.StatePath, err) + } + + return 0, nil + } + + st, err := sandbox.ReadState(g.StatePath) + if err != nil { + return 0, err + } + + // The recorded pid is the sandbox's own init and not the bwrap that + // started it. Killing the outer bwrap does not signal the sandbox + // nested below it. The init is pid 1 of its own pid namespace, and a + // SIGKILL from an ancestor namespace takes the whole namespace with it, + // which is the pid running.stop names when it takes down a gateway that + // failed to come up. + // + // The uplink is not named here. pasta is not recorded anywhere, and the + // tap it created lives in the network namespace that has just gone. + if err := syscall.Kill(st.PID, syscall.SIGKILL); err != nil && !errors.Is(err, syscall.ESRCH) { + return 0, fmt.Errorf("failed to stop the gateway sandbox pid %d: %w", st.PID, err) + } + + // A delivered signal is not a sandbox that has gone. The record has to + // outlive the wait, because the next launch takes this same lock and + // decides by what it finds: removing the record first would let it see + // nothing, and start a replacement while the old pid namespace, its + // outer bwrap and its uplink were still coming down. + if err := waitGone(g.StatePath, stopGrace, stopPoll); err != nil { + return st.PID, err + } + + // A qubesome run at a terminal exits long before the gateway it started + // does, so the goroutine that would have cleared this record has + // usually gone with it. + if err := os.Remove(g.StatePath); err != nil && !errors.Is(err, os.ErrNotExist) { + return st.PID, fmt.Errorf("failed to remove the gateway state %q: %w", g.StatePath, err) + } + + return st.PID, nil +} + +const ( + // stopGrace bounds the wait for a signalled gateway to go. SIGKILL to + // pid 1 of a pid namespace takes the namespace with it and is not + // something the process can put off, so this is a ceiling on the + // kernel finishing rather than an estimate of anything. + stopGrace = 5 * time.Second + + // stopPoll is how often the record is checked while waiting. There is + // nothing to wait on: the gateway is not this process's child, so it + // cannot be reaped here and its going away is not something the + // kernel will report. + stopPoll = 20 * time.Millisecond +) + +// waitGone blocks until the sandbox recorded at path has finished. +// +// sandbox.Exited and not the negation of sandbox.Alive, because a process +// killed by something that is not its parent stays in the table until +// somebody reaps it. Waiting for it to leave /proc would mean waiting for +// a parent that has usually exited long ago. +func waitGone(path string, grace, poll time.Duration) error { + deadline := time.Now().Add(grace) + + for { + if sandbox.Exited(path) { + return nil + } + + if time.Now().After(deadline) { + return fmt.Errorf("timed out after %s waiting for the gateway sandbox to stop", grace) + } + + time.Sleep(poll) + } +} diff --git a/internal/gateway/stop_test.go b/internal/gateway/stop_test.go new file mode 100644 index 0000000..facb5dd --- /dev/null +++ b/internal/gateway/stop_test.go @@ -0,0 +1,237 @@ +// Stopping a real gateway cannot be tested here, for the reasons +// run_test.go gives: no sandbox starts in the development container, so +// there is never one to take down. What is covered is what Stop decides: +// which pid it signals, that it signals it at all, and the two cases where +// there is nothing to signal. +package gateway + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/qubesome/cli/internal/sandbox" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/sys/execabs" +) + +// A session with no gateway in it is the state the caller asked for. +func TestStopWithNoGatewayRunning(t *testing.T) { + t.Parallel() + + pid, err := newSessionGateway(t).Stop() + + require.NoError(t, err) + assert.Zero(t, pid) +} + +// A host that has never started a session has no session directory, and +// asking it to stop a gateway is still not an error. +func TestStopWithoutASessionDir(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + g.Dir = filepath.Join(g.Dir, "session") + g.LockPath = filepath.Join(g.Dir, "gateway.lock") + g.StatePath = filepath.Join(g.Dir, "sandbox-gateway.json") + + pid, err := g.Stop() + + require.NoError(t, err) + assert.Zero(t, pid) +} + +// A record left behind by a gateway that crashed names a pid the kernel may +// since have given to something else. It reads as no gateway, so nothing is +// signalled, and the record goes because reap would have removed it. +func TestStopRemovesARecordLeftBehind(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + state := fmt.Sprintf(`{"pid":%d,"startTime":1}`, os.Getpid()) + require.NoError(t, os.WriteFile(g.StatePath, []byte(state), 0o600)) + + pid, err := g.Stop() + + require.NoError(t, err) + assert.Zero(t, pid) + assert.NoFileExists(t, g.StatePath) +} + +// The pid in the record is the one that is signalled, and the record it came +// from does not outlive it. +func TestStopKillsTheRecordedProcess(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cmd := execabs.CommandContext(ctx, "sleep", "60") + require.NoError(t, cmd.Start()) + + require.NoError(t, sandbox.WriteState(g.StatePath, cmd.Process.Pid)) + + pid, err := g.Stop() + + require.NoError(t, err) + assert.Equal(t, cmd.Process.Pid, pid) + assert.NoFileExists(t, g.StatePath) + + err = cmd.Wait() + require.Error(t, err) + assert.Contains(t, err.Error(), "killed") +} + +// Nothing is signalled twice. The second call finds a record naming a +// process that is gone, which is the crashed gateway case again. +func TestStopIsRepeatable(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cmd := execabs.CommandContext(ctx, "sleep", "60") + require.NoError(t, cmd.Start()) + + require.NoError(t, sandbox.WriteState(g.StatePath, cmd.Process.Pid)) + + _, err := g.Stop() + require.NoError(t, err) + _ = cmd.Wait() + + pid, err := g.Stop() + require.NoError(t, err) + assert.Zero(t, pid) +} + +// The lock Stop takes is the one a launch takes to start a gateway, so it is +// released by the time Stop returns and a launch is not left waiting on it. +func TestStopReleasesTheGatewayLock(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + _, err := g.Stop() + require.NoError(t, err) + + done := make(chan struct{}) + go func() { + defer close(done) + + lock, err := acquire(g.LockPath) + if err == nil { + lock.Close() + } + }() + + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("the gateway lock was still held after Stop returned") + } +} + +// A successful kill is a signal delivered and not a sandbox gone. Stop +// waits, because the next launch takes the same lock and would otherwise +// find no record and start a replacement while the old namespace and its +// uplink were still coming down. +func TestWaitGone(t *testing.T) { + t.Parallel() + + t.Run("returns once the process is gone", func(t *testing.T) { + t.Parallel() + + cmd := execabs.Command("sleep", "60") + require.NoError(t, cmd.Start()) + + path := statePathFor(t, cmd.Process.Pid) + require.NoError(t, cmd.Process.Kill()) + + require.NoError(t, waitGone(path, 5*time.Second, time.Millisecond)) + + _ = cmd.Wait() + }) + + // The one that matters. A gateway is not the child of whatever stops + // it, so nothing reaps it here and it sits as a zombie until its real + // parent, or init, collects it. Waiting for it to leave the process + // table would mean waiting out the whole grace every time. + t.Run("does not wait for a zombie to be reaped", func(t *testing.T) { + t.Parallel() + + cmd := execabs.Command("sleep", "60") + require.NoError(t, cmd.Start()) + + path := statePathFor(t, cmd.Process.Pid) + require.NoError(t, cmd.Process.Kill()) + + // Long enough that a wait on reaping would fail the assertion + // rather than pass it slowly. + start := time.Now() + require.NoError(t, waitGone(path, 30*time.Second, time.Millisecond)) + assert.Less(t, time.Since(start), 5*time.Second) + + _ = cmd.Wait() + }) + + t.Run("gives up on a process that will not go", func(t *testing.T) { + t.Parallel() + + cmd := execabs.Command("sleep", "60") + require.NoError(t, cmd.Start()) + t.Cleanup(func() { _ = cmd.Process.Kill(); _ = cmd.Wait() }) + + err := waitGone(statePathFor(t, cmd.Process.Pid), 50*time.Millisecond, time.Millisecond) + + require.Error(t, err) + assert.Contains(t, err.Error(), "waiting for the gateway sandbox to stop") + }) + + t.Run("a record that names nothing is already gone", func(t *testing.T) { + t.Parallel() + + require.NoError(t, waitGone(filepath.Join(t.TempDir(), "absent.json"), time.Second, time.Millisecond)) + }) +} + +func TestStopWaitsForTheSandboxToGo(t *testing.T) { + t.Parallel() + + g := newSessionGateway(t) + + cmd := execabs.Command("sleep", "60") + require.NoError(t, cmd.Start()) + pid := cmd.Process.Pid + + require.NoError(t, sandbox.WriteState(g.StatePath, pid)) + + got, err := g.Stop() + require.NoError(t, err) + assert.Equal(t, pid, got) + assert.NoFileExists(t, g.StatePath) + + assert.True(t, sandbox.Exited(statePathFor(t, pid)), + "Stop returned while the sandbox was still running") + + _ = cmd.Wait() +} + +// statePathFor writes a record naming pid, so a test can ask about a +// process whose own record Stop has already removed. +func statePathFor(t *testing.T, pid int) string { + t.Helper() + + path := filepath.Join(t.TempDir(), "sandbox.json") + require.NoError(t, sandbox.WriteState(path, pid)) + + return path +} diff --git a/internal/gateway/summary.go b/internal/gateway/summary.go new file mode 100644 index 0000000..3b1c19a --- /dev/null +++ b/internal/gateway/summary.go @@ -0,0 +1,104 @@ +package gateway + +import ( + "bufio" + "errors" + "fmt" + "os" + "slices" + "strings" +) + +// errNoLog reports a session with no gateway log to read. It is a distinct +// error because a caller diagnosing a host has to tell "the gateway said +// nothing worth reporting" from "there was nothing to read". +var errNoLog = errors.New("no gateway log") + +// maxDeniedHosts bounds how many denied hosts a summary names. A policy +// that denies a great deal is working, and a diagnosis of it should say so +// in a line rather than reproduce the log. +const maxDeniedHosts = 5 + +// LogSummary is what a gateway log says has happened. +// +// It is deliberately small. It exists so that qubesome doctor can say +// whether the gateway has been refusing connections or failing to make +// them, without reading the log for the user or growing an opinion about +// what a policy ought to allow. +type LogSummary struct { + // Decisions is how many connections the proxy classified. + Decisions int + + // Denied is how many of those it refused. A denial is the policy + // working, so this is not a count of faults. It is the answer to why + // a workload could not reach something. + Denied int + + // DeniedHosts names the distinct hosts that were denied, in the order + // they were first refused, at most maxDeniedHosts of them. + DeniedHosts []string + + // Errors is how many lines the gateway logged at error level. Unlike + // a denial, each of these is something that did not work. + Errors int + + // LastError is the message of the most recent of them. + LastError string +} + +// Summarise reads the gateway log at path and reports what it says. +// +// A line it cannot parse contributes nothing rather than failing the read. +// The log's shape belongs to the gateway, which is a separate component on +// its own release cycle, so a summary of it degrades to saying less rather +// than to refusing to say anything. +func Summarise(path string) (LogSummary, error) { + f, err := os.Open(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return LogSummary{}, fmt.Errorf("%w at %s", errNoLog, path) + } + + return LogSummary{}, fmt.Errorf("failed to open the gateway log %q: %w", path, err) + } + defer f.Close() + + var s LogSummary + + scanner := bufio.NewScanner(f) + scanner.Buffer(make([]byte, 0, bufio.MaxScanTokenSize), maxLineLen) + + for scanner.Scan() { + s.read(scanner.Text()) + } + if err := scanner.Err(); err != nil { + return LogSummary{}, fmt.Errorf("failed to read the gateway log %q: %w", path, err) + } + + return s, nil +} + +// read folds one log line into the summary. +func (s *LogSummary) read(line string) { + if strings.EqualFold(logField(line, fieldLevel), "error") { + s.Errors++ + s.LastError = logField(line, "msg") + } + + action := logField(line, fieldAction) + if action == "" { + return + } + s.Decisions++ + + if action != "deny" { + return + } + s.Denied++ + + host := logField(line, fieldHost) + if host == "" || len(s.DeniedHosts) == maxDeniedHosts || slices.Contains(s.DeniedHosts, host) { + return + } + s.DeniedHosts = append(s.DeniedHosts, host) +} diff --git a/internal/gateway/summary_test.go b/internal/gateway/summary_test.go new file mode 100644 index 0000000..4830059 --- /dev/null +++ b/internal/gateway/summary_test.go @@ -0,0 +1,73 @@ +package gateway + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSummarise(t *testing.T) { + t.Parallel() + + path := writeLog(t, + `level=INFO msg="starting gateway"`, + `level=INFO msg="proxy decision" workload=cli-llm-work host=api.anthropic.com action=splice`, + `level=INFO msg="proxy decision" workload=chrome-work host=ads.example.com action=deny reason="policy denied host"`, + `level=INFO msg="proxy decision" workload=chrome-work host=eu.ads.example.com action=deny reason="policy denied host"`, + `level=INFO msg="proxy decision" workload=chrome-work host=ads.example.com action=deny reason="policy denied host"`, + `level=WARN msg="splice dial failed" workload=chrome-work host=slow.example`, + `level=ERROR msg="original destination lookup failed" plane=proxy`, + `level=INFO msg="proxy decision" workload=cli-llm-work host=api.github.com action=inject`, + ) + + got, err := Summarise(path) + require.NoError(t, err) + + assert.Equal(t, 5, got.Decisions, "every proxy decision counts, whatever its verdict") + assert.Equal(t, 3, got.Denied) + assert.Equal(t, []string{"ads.example.com", "eu.ads.example.com"}, got.DeniedHosts, + "a host denied twice is named once") + assert.Equal(t, 1, got.Errors, "a warning is not an error") + assert.Equal(t, "original destination lookup failed", got.LastError) +} + +func TestSummariseAQuietLog(t *testing.T) { + t.Parallel() + + got, err := Summarise(writeLog(t, `level=INFO msg="starting gateway"`)) + require.NoError(t, err) + + assert.Zero(t, got.Decisions) + assert.Zero(t, got.Denied) + assert.Empty(t, got.DeniedHosts) + assert.Zero(t, got.Errors) +} + +func TestSummariseWithoutALog(t *testing.T) { + t.Parallel() + + _, err := Summarise(filepath.Join(t.TempDir(), "absent.log")) + require.Error(t, err) + assert.ErrorIs(t, err, errNoLog) +} + +// A gateway that denies a great many hosts must not turn a diagnosis into +// a list of them. +func TestSummariseCapsTheHostsItNames(t *testing.T) { + t.Parallel() + + lines := make([]string, 0, maxDeniedHosts*2) + for i := range maxDeniedHosts * 2 { + lines = append(lines, + `level=INFO msg="proxy decision" workload=w-p action=deny host=h`+ + string(rune('a'+i))+`.example`) + } + + got, err := Summarise(writeLog(t, lines...)) + require.NoError(t, err) + + assert.Equal(t, maxDeniedHosts*2, got.Denied, "the count is of every denial") + assert.Len(t, got.DeniedHosts, maxDeniedHosts, "the names are capped") +} diff --git a/internal/profiles/display.go b/internal/profiles/display.go index d1ef6b9..ca5223d 100644 --- a/internal/profiles/display.go +++ b/internal/profiles/display.go @@ -136,8 +136,16 @@ func xwaylandArgs(p DisplayParams) ([]string, error) { // The window manager, and everything it launches, must not inherit a // path to the compositor. A client that reaches the Wayland socket // bypasses Xwayland and the isolation set above. + // + // DISPLAY names this profile's server rather than being left to what + // xwayland-run exports. The profile container's own DISPLAY is the + // host's, because that is what the compositor presents through, and + // the whole of /tmp/.X11-unix is mounted so that its socket is + // reachable. Anything under here that inherited that would be talking + // to the host session instead of to the profile. args = append(args, "--", "env", "-u", "WAYLAND_DISPLAY", + "DISPLAY=:"+strconv.Itoa(int(p.Display)), "XDG_RUNTIME_DIR="+appRuntimeDir, "XAUTHORITY="+clientAuthFile) diff --git a/internal/profiles/display_test.go b/internal/profiles/display_test.go index 3e30a2b..82059be 100644 --- a/internal/profiles/display_test.go +++ b/internal/profiles/display_test.go @@ -6,6 +6,7 @@ import ( "net" "os" "path/filepath" + "slices" "testing" "time" @@ -80,7 +81,7 @@ func TestXwaylandArgs(t *testing.T) { "-tst", "-nolisten", "tcp", "--", - "env", "-u", "WAYLAND_DISPLAY", + "env", "-u", "WAYLAND_DISPLAY", "DISPLAY=:11", "XDG_RUNTIME_DIR=/run/user/1000", "XAUTHORITY=/home/xorg-user/.Xauthority", "dbus-run-session", "awesome", @@ -106,7 +107,7 @@ func TestXwaylandArgs(t *testing.T) { "-nolisten", "tcp", "-verbose", "9", "--", - "env", "-u", "WAYLAND_DISPLAY", + "env", "-u", "WAYLAND_DISPLAY", "DISPLAY=:11", "XDG_RUNTIME_DIR=/run/user/1000", "XAUTHORITY=/home/xorg-user/.Xauthority", "dbus-run-session", "awesome", @@ -314,3 +315,32 @@ func TestCompositorStatusDistinguishesACleanExitFromStillRunning(t *testing.T) { require.Len(t, exit, 1) }) } + +// The profile container reaches the host X server: its DISPLAY names the +// host session and the whole of /tmp/.X11-unix is mounted, which is how +// the compositor presents the profile at all. The window manager must not +// inherit that. It is given the profile's own display by name rather than +// left to whatever xwayland-run happens to export, for the same reason it +// is given the profile's cookie and has WAYLAND_DISPLAY taken away. +func TestXwaylandArgsNamesTheProfileDisplay(t *testing.T) { + t.Parallel() + + got, err := xwaylandArgs(DisplayParams{ + Display: 11, + Geometry: "1920x1080", + AuthFile: "/home/xorg-user/.Xserver", + WindowManager: "exec awesome", + }) + require.NoError(t, err) + + // After the -- separator, so it is the window manager's environment + // and not an argument to Xwayland. + sep := slices.Index(got, "--") + require.NotEqual(t, -1, sep, "the window manager must be separated from the server arguments") + require.Contains(t, got[sep:], "DISPLAY=:11") + + wm := slices.Index(got, "awesome") + require.NotEqual(t, -1, wm, "the window manager must still be run") + require.Less(t, slices.Index(got, "DISPLAY=:11"), wm, + "the display has to be set before the command, or env takes it as an argument") +} diff --git a/internal/profiles/profiles.go b/internal/profiles/profiles.go index 6eea33f..a093467 100644 --- a/internal/profiles/profiles.go +++ b/internal/profiles/profiles.go @@ -34,6 +34,7 @@ import ( "github.com/qubesome/cli/internal/util/gpu" "github.com/qubesome/cli/internal/util/mtls" "github.com/qubesome/cli/internal/util/resolution" + "github.com/qubesome/cli/internal/util/tz" "github.com/qubesome/cli/internal/util/xauth" "github.com/qubesome/cli/internal/util/xkb" "github.com/qubesome/cli/pkg/inception" @@ -571,8 +572,8 @@ func createMagicCookie(profile *types.Profile) error { // and the desktop files are mounted, so the environment and the mount // list cannot disagree. bwrap applies --setenv in order, so these are the // values that survive. -func sandboxEnv(bundle images.Bundle, ca, cert, key []byte) []string { - const extra = 6 +func sandboxEnv(bundle images.Bundle, ca, cert, key []byte, timezone string) []string { + const extra = 7 // The compositor decides the keymap for everything in the profile, so // the host's layout is carried in here rather than anywhere nearer @@ -591,6 +592,13 @@ func sandboxEnv(bundle images.Bundle, ca, cert, key []byte) []string { env = append(env, bundle.Env...) env = append(env, keymap...) + // An empty value is not the same as no value here: a C library reads + // TZ="" as UTC, so a host with no timezone to give has to leave the + // image's alone rather than say nothing in a way that means UTC. + if timezone != "" { + env = append(env, "TZ="+timezone) + } + return append(env, "HOME="+profileHome, "USER="+profileUser, @@ -736,8 +744,23 @@ func createNewDisplay(bundle images.Bundle, ca, cert, key []byte, profile *types return nil, err } + // The host's timezone is shared as a zone file under its own name, + // and TZ below is what points the sandbox at it. It is deliberately + // not mounted on /etc/localtime: an image ships that as a symlink, + // and bubblewrap 0.12.0 refuses to mount on one. Releases before it + // followed the link and mounted on its target, which is why sharing + // /etc/localtime worked until 0.12.0 landed. + zone := tz.Host() + + // A profile that names a timezone means it for everything inside, + // the window manager's own clock included, whatever the host is set + // to. + timezone := profile.Timezone + if timezone == "" { + timezone = zone.TZ + } + mounts := []sandbox.Mount{ - {Src: "/etc/localtime", Dst: "/etc/localtime", ReadOnly: true}, {Src: x11Dir, Dst: "/tmp/.X11-unix"}, {Src: socket, Dst: "/tmp/qube.sock", ReadOnly: true}, {Src: server, Dst: profileHome + "/.Xserver"}, @@ -745,6 +768,14 @@ func createNewDisplay(bundle images.Bundle, ca, cert, key []byte, profile *types {Src: binPath, Dst: files.InProfileBinary, ReadOnly: true}, } + if zone.HostPath != "" { + mounts = append(mounts, sandbox.Mount{ + Src: zone.HostPath, + Dst: zone.SandboxPath, + ReadOnly: true, + }) + } + for _, p := range profile.Paths { p = env.Expand(p) @@ -821,7 +852,7 @@ func createNewDisplay(bundle images.Bundle, ca, cert, key []byte, profile *types } } - senv := sandboxEnv(bundle, ca, cert, key) + senv := sandboxEnv(bundle, ca, cert, key, timezone) // The profile runs its own compositor, so it needs nothing from the // host session beyond the display socket mounted above. The session diff --git a/internal/profiles/profiles_test.go b/internal/profiles/profiles_test.go index 284c91c..c477e85 100644 --- a/internal/profiles/profiles_test.go +++ b/internal/profiles/profiles_test.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" "testing" @@ -57,7 +58,7 @@ func TestSandboxEnvNamesTheProfileUser(t *testing.T) { t.Setenv("DISPLAY", ":0") bundle := images.Bundle{Env: []string{"PATH=/usr/bin", "HOME=/root"}} - senv := sandboxEnv(bundle, []byte("ca"), []byte("cert"), []byte("key")) + senv := sandboxEnv(bundle, []byte("ca"), []byte("cert"), []byte("key"), "") assert.Equal(t, []string{"PATH=/usr/bin", "HOME=/root"}, senv[:2], "the image environment must come first") @@ -72,12 +73,35 @@ func TestSandboxEnvNamesTheProfileUser(t *testing.T) { func TestSandboxEnvWithAnEmptyImageEnvironment(t *testing.T) { t.Setenv("DISPLAY", ":0") - senv := sandboxEnv(images.Bundle{}, nil, nil, nil) + senv := sandboxEnv(images.Bundle{}, nil, nil, nil, "") assert.Equal(t, "/home/xorg-user", lastEnv(senv, "HOME")) assert.Equal(t, "xorg-user", lastEnv(senv, "USER")) } +// The profile sandbox used to read the host timezone out of the +// /etc/localtime shared with it. Nothing is mounted there any more, so +// TZ is what carries it to the window manager's own clock. +func TestSandboxEnvCarriesTheTimezone(t *testing.T) { + t.Setenv("DISPLAY", ":0") + + senv := sandboxEnv(images.Bundle{}, nil, nil, nil, "Europe/London") + + assert.Equal(t, "Europe/London", lastEnv(senv, "TZ")) +} + +// An empty TZ is not the same as no TZ: a C library reads one as UTC, +// so a host with no timezone to give must leave the image's alone. +func TestSandboxEnvWithoutATimezone(t *testing.T) { + t.Setenv("DISPLAY", ":0") + + senv := sandboxEnv(images.Bundle{}, nil, nil, nil, "") + + assert.False(t, slices.ContainsFunc(senv, func(e string) bool { + return strings.HasPrefix(e, "TZ=") + })) +} + // lastEnv returns the value of the last assignment to name, which is the // one bwrap keeps. func lastEnv(env []string, name string) string { diff --git a/internal/runners/bwrap/proxyenv_test.go b/internal/runners/bwrap/proxyenv_test.go new file mode 100644 index 0000000..e23d257 --- /dev/null +++ b/internal/runners/bwrap/proxyenv_test.go @@ -0,0 +1,50 @@ +package bwrap + +import ( + "testing" + + "github.com/qubesome/cli/internal/images" + "github.com/qubesome/cli/internal/types" + "github.com/stretchr/testify/assert" +) + +// A workload that has a gateway is told where to ask it for a tunnel. One +// without a gateway has nothing to be told, and an empty variable would +// read as an endpoint of nothing at all. +func TestWorkloadEnvNamesTheGatewayProxy(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + proxy string + want bool + }{ + {"attached to a gateway", "10.111.0.1:3128", true}, + {"no gateway", "", false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + env := workloadEnv(input{ + Workload: types.EffectiveWorkload{ + Name: "w-p", + Profile: &types.Profile{Name: "p"}, + Workload: types.Workload{}, + }, + Bundle: images.Bundle{}, + GatewayProxy: tc.proxy, + }) + + if tc.want { + assert.Contains(t, env, "QUBESOME_GATEWAY_PROXY="+tc.proxy) + + return + } + for _, e := range env { + assert.NotContains(t, e, "QUBESOME_GATEWAY_PROXY") + } + }) + } +} diff --git a/internal/runners/bwrap/run.go b/internal/runners/bwrap/run.go index ec144fa..fa3ddcb 100644 --- a/internal/runners/bwrap/run.go +++ b/internal/runners/bwrap/run.go @@ -22,6 +22,7 @@ import ( "github.com/qubesome/cli/internal/util/dbus" "github.com/qubesome/cli/internal/util/env" "github.com/qubesome/cli/internal/util/gpu" + "github.com/qubesome/cli/internal/util/tz" "golang.org/x/sys/execabs" ) @@ -104,6 +105,16 @@ func Run(ew types.EffectiveWorkload, cfg *types.Config) error { return err } + // The endpoint the workload asks for a tunnel on, which is known here + // because the address was allocated before the sandbox was built. + if att != nil { + proxy, err := att.ProxyAddr() + if err != nil { + return err + } + in.GatewayProxy = proxy + } + spec, err := buildSpec(in) if err != nil { return err @@ -372,7 +383,7 @@ func resolve(ew types.EffectiveWorkload, gw bool) (input, error) { ShmDir: shmDir, CookiePath: cookiePath, SocketPath: socketPath, - Localtime: localtime(), + Zone: tz.Host(), USBDevices: usbDevices, Paths: mappedPaths(wl.HostAccess.Paths), } @@ -478,32 +489,6 @@ func resolveMime(in *input) error { return nil } -// localtime returns /etc/localtime and, when it is a symlink, the file it -// points at. -// -// The link on its own resolves to nothing inside the sandbox, so both are -// shared. -func localtime() []string { - const file = "/etc/localtime" - - if _, err := os.Stat(file); err != nil { - return nil - } - - paths := make([]string, 0, 2) - paths = append(paths, file) - - target, err := os.Readlink(file) - if err != nil { - return paths - } - if !filepath.IsAbs(target) { - target = filepath.Join(filepath.Dir(file), target) - } - - return append(paths, target) -} - // mappedPaths expands the workload's mapped directories and creates the // host side of each. // diff --git a/internal/runners/bwrap/run_test.go b/internal/runners/bwrap/run_test.go index 6f69dbf..1e937e9 100644 --- a/internal/runners/bwrap/run_test.go +++ b/internal/runners/bwrap/run_test.go @@ -13,6 +13,7 @@ import ( "github.com/qubesome/cli/internal/sandbox" "github.com/qubesome/cli/internal/types" "github.com/qubesome/cli/internal/util/gpu" + "github.com/qubesome/cli/internal/util/tz" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -69,7 +70,11 @@ func plainInput() input { ShmDir: "/run/user/1000/qubesome/work/shm/chrome", CookiePath: "/run/user/1000/qubesome/work/.Xclient-cookie", SocketPath: "/run/user/1000/qubesome/work/qube.sock", - Localtime: []string{"/etc/localtime", "/usr/share/zoneinfo/Europe/London"}, + Zone: tz.Zone{ + HostPath: "/usr/share/zoneinfo/Europe/London", + SandboxPath: "/usr/share/zoneinfo/Europe/London", + TZ: "Europe/London", + }, } } @@ -266,15 +271,65 @@ func TestSpecIsolatedRunUser(t *testing.T) { indexOfArg(args, "--ro-bind", filepath.Join(in.ProfileDir, "machine-id"))) } -// /etc/localtime is usually a symlink, and the link alone resolves to -// nothing inside the sandbox. -func TestSpecSharesLocaltimeAndItsTarget(t *testing.T) { +// An image ships /etc/localtime as a symlink, and bubblewrap 0.12.0 +// refuses to mount on one. The host's zone reaches the sandbox as a +// zone file under its own name plus TZ, so nothing mounts there at all. +func TestSpecNeverMountsOnLocaltime(t *testing.T) { t.Parallel() - args := render(t, plainInput()) + assert.NotContains(t, render(t, plainInput()), "/etc/localtime") +} + +func TestSpecSharesTheHostZoneAndNamesIt(t *testing.T) { + t.Parallel() + + in := plainInput() + in.Workload.Profile.Timezone = "" + in.Zone = tz.Zone{ + HostPath: "/etc/zoneinfo/Europe/London", + SandboxPath: "/usr/share/zoneinfo/Europe/London", + TZ: "Europe/London", + } + + args := render(t, in) + + i := indexOfArg(args, "--ro-bind", "/etc/zoneinfo/Europe/London") + require.NotEqual(t, -1, i) + assert.Equal(t, "/usr/share/zoneinfo/Europe/London", args[i+2]) + + j := indexOfArg(args, "--setenv", "TZ") + require.NotEqual(t, -1, j) + assert.Equal(t, "Europe/London", args[j+2]) +} + +// A profile that names a timezone means it, whatever the host is set to. +func TestSpecPrefersTheProfileTimezone(t *testing.T) { + t.Parallel() + + in := plainInput() + in.Workload.Profile.Timezone = "America/New_York" + + args := render(t, in) + + i := indexOfArg(args, "--setenv", "TZ") + require.NotEqual(t, -1, i) + assert.Equal(t, "America/New_York", args[i+2]) + assert.Equal(t, 1, countArg(args, "--setenv", "TZ")) +} + +// A host with no timezone to give leaves the sandbox on the image's own, +// which is what the container runner did before it. +func TestSpecWithoutATimezone(t *testing.T) { + t.Parallel() + + in := plainInput() + in.Workload.Profile.Timezone = "" + in.Zone = tz.Zone{} + + args := render(t, in) - assert.NotEqual(t, -1, indexOfArg(args, "--ro-bind", "/etc/localtime")) - assert.NotEqual(t, -1, indexOfArg(args, "--ro-bind", "/usr/share/zoneinfo/Europe/London")) + assert.Equal(t, -1, indexOfArg(args, "--setenv", "TZ")) + assert.NotContains(t, args, "/usr/share/zoneinfo/Europe/London") } // There is no uplink in this stage, so a workload with anything short of diff --git a/internal/runners/bwrap/spec.go b/internal/runners/bwrap/spec.go index d8a2abe..6c3c1aa 100644 --- a/internal/runners/bwrap/spec.go +++ b/internal/runners/bwrap/spec.go @@ -21,6 +21,7 @@ import ( "github.com/qubesome/cli/internal/sandbox" "github.com/qubesome/cli/internal/types" "github.com/qubesome/cli/internal/util/gpu" + "github.com/qubesome/cli/internal/util/tz" ) // runUserDir is the runtime directory a workload sees. Every workload runs @@ -88,10 +89,10 @@ type input struct { // workload does not handle mime types. HomeDir string - // Localtime is /etc/localtime and, when that is a symlink, the file it - // points at. Both are needed: the link alone resolves to nothing - // inside the sandbox. - Localtime []string + // Zone is the host's timezone. It is the zero value when the host + // has none to give, and it is ignored when the profile names a + // timezone of its own. + Zone tz.Zone // VideoDevices are the /dev/video* nodes found on the host. VideoDevices []string @@ -110,6 +111,11 @@ type input struct { // created on the host. Paths []sandbox.Mount + // GatewayProxy is where the workload asks the gateway for a tunnel to + // a host it may reach on a port the transparent path does not carry. + // Empty for a workload with no gateway, which has nowhere to ask. + GatewayProxy string + // HostEnv holds the host variables a workload on the host dbus reads. // The container runners named them and let the runtime copy the // values across. bwrap clears the environment instead, so the values @@ -354,8 +360,17 @@ func workloadMounts(in input) []sandbox.Mount { var mounts []sandbox.Mount - for _, p := range in.Localtime { - mounts = append(mounts, sandbox.Mount{Src: p, Dst: p, ReadOnly: true}) + // The zone file lands under its own name rather than on + // /etc/localtime, which an image ships as a symlink and bubblewrap + // 0.12.0 refuses to mount on. TZ below is what points the sandbox at + // it, so an image whose timezone database already holds the name + // only gains the host's copy of the same file. + if in.Zone.HostPath != "" { + mounts = append(mounts, sandbox.Mount{ + Src: in.Zone.HostPath, + Dst: in.Zone.SandboxPath, + ReadOnly: true, + }) } mounts = append(mounts, sandbox.Mount{Src: in.ShmDir, Dst: "/dev/shm"}) @@ -470,7 +485,7 @@ func workloadEnv(in input) []string { wl := in.Workload.Workload profile := in.Workload.Profile - const extra = 8 + const extra = 9 env := make([]string, 0, len(in.Bundle.Env)+len(in.HostEnv)+extra) env = append(env, in.Bundle.Env...) @@ -480,8 +495,22 @@ func workloadEnv(in input) []string { "QUBESOME_PROFILE="+profile.Name, ) - if profile.Timezone != "" { - env = append(env, "TZ="+profile.Timezone) + // Only when there is one. An empty value would read as an endpoint + // that is there and is nothing, and a workload with no gateway has + // nowhere to ask for a tunnel at all. + if in.GatewayProxy != "" { + env = append(env, "QUBESOME_GATEWAY_PROXY="+in.GatewayProxy) + } + + // A profile that names a timezone means it, whatever the host is set + // to. Otherwise the workload follows the host, which it used to do + // by reading the /etc/localtime shared with it. + timezone := profile.Timezone + if timezone == "" { + timezone = in.Zone.TZ + } + if timezone != "" { + env = append(env, "TZ="+timezone) } env = append(env, in.HostEnv...) diff --git a/internal/runners/bwrap/testdata/granted.golden b/internal/runners/bwrap/testdata/granted.golden index e06bd0b..2d19da2 100644 --- a/internal/runners/bwrap/testdata/granted.golden +++ b/internal/runners/bwrap/testdata/granted.golden @@ -59,9 +59,6 @@ kali-vpn-pentest /dev/hidraw9 /dev/hidraw9 --ro-bind -/etc/localtime -/etc/localtime ---ro-bind /usr/share/zoneinfo/Europe/London /usr/share/zoneinfo/Europe/London --bind diff --git a/internal/runners/bwrap/testdata/hostnet.golden b/internal/runners/bwrap/testdata/hostnet.golden index 9ffd706..efc8f3c 100644 --- a/internal/runners/bwrap/testdata/hostnet.golden +++ b/internal/runners/bwrap/testdata/hostnet.golden @@ -32,9 +32,6 @@ chrome-work /dev/dri /dev/dri --ro-bind -/etc/localtime -/etc/localtime ---ro-bind /usr/share/zoneinfo/Europe/London /usr/share/zoneinfo/Europe/London --bind diff --git a/internal/runners/bwrap/testdata/plain.golden b/internal/runners/bwrap/testdata/plain.golden index fb1feb8..34967a2 100644 --- a/internal/runners/bwrap/testdata/plain.golden +++ b/internal/runners/bwrap/testdata/plain.golden @@ -33,9 +33,6 @@ chrome-work /dev/dri /dev/dri --ro-bind -/etc/localtime -/etc/localtime ---ro-bind /usr/share/zoneinfo/Europe/London /usr/share/zoneinfo/Europe/London --bind diff --git a/internal/runners/bwrap/testdata/supervised.golden b/internal/runners/bwrap/testdata/supervised.golden index dd82ae5..de396ae 100644 --- a/internal/runners/bwrap/testdata/supervised.golden +++ b/internal/runners/bwrap/testdata/supervised.golden @@ -33,9 +33,6 @@ chrome-work /dev/dri /dev/dri --ro-bind -/etc/localtime -/etc/localtime ---ro-bind /usr/share/zoneinfo/Europe/London /usr/share/zoneinfo/Europe/London --bind diff --git a/internal/sandbox/exited_test.go b/internal/sandbox/exited_test.go new file mode 100644 index 0000000..81bd017 --- /dev/null +++ b/internal/sandbox/exited_test.go @@ -0,0 +1,75 @@ +package sandbox + +import ( + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func recordOf(t *testing.T, pid int) string { + t.Helper() + + path := filepath.Join(t.TempDir(), "sandbox.json") + require.NoError(t, WriteState(path, pid)) + + return path +} + +func TestExited(t *testing.T) { + t.Parallel() + + t.Run("a running process has not", func(t *testing.T) { + t.Parallel() + + cmd := exec.CommandContext(t.Context(), "sleep", "60") + require.NoError(t, cmd.Start()) + t.Cleanup(func() { _ = cmd.Process.Kill(); _, _ = cmd.Process.Wait() }) + + assert.False(t, Exited(recordOf(t, cmd.Process.Pid))) + }) + + // A process killed by something that is not its parent is left for + // whoever reaps it. Until then its /proc entry is there and its start + // time still matches, so a liveness check alone reads it as running + // when it is only waiting to be collected. + t.Run("a zombie has", func(t *testing.T) { + t.Parallel() + + cmd := exec.CommandContext(t.Context(), "sleep", "60") + require.NoError(t, cmd.Start()) + pid := cmd.Process.Pid + path := recordOf(t, pid) + + require.NoError(t, cmd.Process.Kill()) + require.Eventually(t, func() bool { return Exited(path) }, 5*time.Second, 10*time.Millisecond, + "a killed process nothing has reaped must read as exited") + + // Still unreaped, so Alive is what it was: the two disagree, and + // that disagreement is the whole point. + assert.True(t, Alive(path), "the record still names a process /proc knows about") + + _, _ = cmd.Process.Wait() + }) + + t.Run("a process that is gone has", func(t *testing.T) { + t.Parallel() + + cmd := exec.CommandContext(t.Context(), "true") + require.NoError(t, cmd.Start()) + pid := cmd.Process.Pid + path := recordOf(t, pid) + _ = cmd.Wait() + + assert.True(t, Exited(path)) + }) + + t.Run("no record at all has", func(t *testing.T) { + t.Parallel() + + assert.True(t, Exited(filepath.Join(t.TempDir(), "absent.json"))) + }) +} diff --git a/internal/sandbox/state.go b/internal/sandbox/state.go index b31dc87..9cc8c48 100644 --- a/internal/sandbox/state.go +++ b/internal/sandbox/state.go @@ -87,9 +87,62 @@ func Alive(path string) bool { return st == s.StartTime } +// Exited reports whether the sandbox recorded at path has finished. +// +// It is not the negation of Alive, and the difference is a process that +// has been killed and not yet reaped. A zombie keeps its /proc entry and +// its start time still matches, so Alive reads it as running when it is +// only waiting to be collected. Nothing is being served by then, and +// something waiting for a sandbox to go would otherwise wait on a parent +// that may never come: the process that started a gateway is usually a +// qubesome run that exited at its terminal long ago. +// +// No record, or one naming a process /proc no longer has, is exited too. +// Both mean the same thing to a caller waiting for one to be gone. +func Exited(path string) bool { + s, err := ReadState(path) + if err != nil { + return true + } + + data, err := os.ReadFile(procStat(s.PID)) + if err != nil { + return true + } + + st, err := parseStartTime(string(data)) + if err != nil || st != s.StartTime { + // A start time that no longer matches is a different process + // wearing the same pid, so the recorded one has gone. + return true + } + + return parseZombie(string(data)) +} + +// parseZombie reports whether a stat line describes a process that has +// exited and is waiting to be reaped. +// +// Field 3 is the state character, and is the first after the command name +// for the reason parseStartTime counts from there. +func parseZombie(line string) bool { + i := strings.LastIndex(line, ")") + if i < 0 { + return false + } + + fields := strings.Fields(line[i+1:]) + + return len(fields) > 0 && fields[0] == "Z" +} + +func procStat(pid int) string { + return "/proc/" + strconv.Itoa(pid) + "/stat" +} + // startTime returns the start time of a process in clock ticks since boot. func startTime(pid int) (uint64, error) { - path := "/proc/" + strconv.Itoa(pid) + "/stat" + path := procStat(pid) data, err := os.ReadFile(path) if err != nil { diff --git a/internal/util/tz/tz.go b/internal/util/tz/tz.go new file mode 100644 index 0000000..13ef274 --- /dev/null +++ b/internal/util/tz/tz.go @@ -0,0 +1,104 @@ +// Package tz carries the host's timezone into a sandbox. +package tz + +import ( + "log/slog" + "path" + "path/filepath" + "regexp" + "strings" +) + +// hostLocaltime is where a host records the zone it is set to. +const hostLocaltime = "/etc/localtime" + +// sandboxZoneinfo is the directory a C library searches for the zone TZ +// names, and it is the same one on every distribution whatever the host +// keeps its own database under. +const sandboxZoneinfo = "/usr/share/zoneinfo" + +// unnamedZone is where a host zone file that names no zone is shared. No +// timezone database ships a file called localtime, so this destination +// cannot land on top of a real zone. +const unnamedZone = sandboxZoneinfo + "/localtime" + +// namePattern is an IANA zone name: slash separated components of the +// characters tzdata uses in its file names, such as Europe/London, +// America/Argentina/Buenos_Aires, Etc/GMT+1 or plain UTC. +// +// A path that does not match names no zone. Passing one to TZ would be +// worse than not naming a zone at all, because a C library that cannot +// read TZ as a zone name reads it as a POSIX rule instead, and every +// rule it fails to parse leaves the sandbox on UTC without saying so. +var namePattern = regexp.MustCompile(`^[A-Za-z0-9_+-]+(?:/[A-Za-z0-9_+-]+)*$`) + +// Zone is the host's timezone in the form a sandbox needs it. +type Zone struct { + // HostPath is the zone file on the host, with every symlink already + // resolved. It is empty when the host has no timezone to give, which + // is the only field a caller needs to test. + HostPath string + + // SandboxPath is where HostPath is shared inside the sandbox. It is + // never /etc/localtime: an image almost always ships that as a + // symlink, and bubblewrap 0.12.0 refuses to mount on one. Releases + // before it followed the link and mounted on its target, which is + // why sharing /etc/localtime worked until 0.12.0 landed. + SandboxPath string + + // TZ is the value of the TZ environment variable, naming the zone + // that SandboxPath holds. + // + // This is what actually gives the sandbox the host's time, since + // /etc/localtime inside it still comes from the image. Every C + // library reads TZ ahead of /etc/localtime, and so do the runtimes + // that resolve a zone themselves rather than through one. + TZ string +} + +// Host returns the host's timezone, and the zero Zone when the host has +// none to give. +func Host() Zone { + return zoneOf(hostLocaltime) +} + +func zoneOf(localtime string) Zone { + // The link is followed here rather than shared as it is, so that a + // host naming its zone through one of the database's own aliases, + // GB for Europe/London, shares the file under the name every image + // has it under. + file, err := filepath.EvalSymlinks(localtime) + if err != nil { + slog.Debug("no host timezone to share", "path", localtime, "error", err) + return Zone{} + } + + name, ok := nameOf(file) + if !ok { + // A host that copied its zone file into place instead of + // linking to one has a timezone but no name for it. TZ takes + // the file directly, which loses only the name: the offsets and + // abbreviations all come out of the file either way. + return Zone{HostPath: file, SandboxPath: unnamedZone, TZ: ":" + unnamedZone} + } + + return Zone{HostPath: file, SandboxPath: path.Join(sandboxZoneinfo, name), TZ: name} +} + +// nameOf reads the zone name out of a resolved zone file's path, which +// is whatever follows the timezone database directory. +func nameOf(file string) (string, bool) { + const database = "/zoneinfo/" + + i := strings.LastIndex(file, database) + if i < 0 { + return "", false + } + + name := file[i+len(database):] + if !namePattern.MatchString(name) { + return "", false + } + + return name, true +} diff --git a/internal/util/tz/tz_test.go b/internal/util/tz/tz_test.go new file mode 100644 index 0000000..61d965e --- /dev/null +++ b/internal/util/tz/tz_test.go @@ -0,0 +1,151 @@ +package tz + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestZoneOf(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + // zoneFile is a regular file created under the root. Empty + // creates none, which leaves a link dangling. + zoneFile string + // link is what etc/localtime points at. Empty makes it a + // regular file instead. + link string + // wantHost is relative to the root, and empty means the host + // has no timezone to give. + wantHost string + wantSandbox string + wantTZ string + }{ + { + name: "a named zone", + zoneFile: "usr/share/zoneinfo/Europe/London", + link: "../usr/share/zoneinfo/Europe/London", + wantHost: "usr/share/zoneinfo/Europe/London", + wantSandbox: "/usr/share/zoneinfo/Europe/London", + wantTZ: "Europe/London", + }, + { + name: "a name of more than two components", + zoneFile: "usr/share/zoneinfo/America/Argentina/Buenos_Aires", + link: "../usr/share/zoneinfo/America/Argentina/Buenos_Aires", + wantHost: "usr/share/zoneinfo/America/Argentina/Buenos_Aires", + wantSandbox: "/usr/share/zoneinfo/America/Argentina/Buenos_Aires", + wantTZ: "America/Argentina/Buenos_Aires", + }, + { + name: "a name of one component", + zoneFile: "usr/share/zoneinfo/UTC", + link: "../usr/share/zoneinfo/UTC", + wantHost: "usr/share/zoneinfo/UTC", + wantSandbox: "/usr/share/zoneinfo/UTC", + wantTZ: "UTC", + }, + { + name: "a database the host does not keep under /usr/share", + zoneFile: "etc/zoneinfo/Europe/London", + link: "zoneinfo/Europe/London", + wantHost: "etc/zoneinfo/Europe/London", + wantSandbox: "/usr/share/zoneinfo/Europe/London", + wantTZ: "Europe/London", + }, + { + name: "a zone file copied into place, which names nothing", + wantHost: "etc/localtime", + wantSandbox: "/usr/share/zoneinfo/localtime", + wantTZ: ":/usr/share/zoneinfo/localtime", + }, + { + name: "a name no timezone database would have written", + zoneFile: "usr/share/zoneinfo/Europe/Lon don", + link: "../usr/share/zoneinfo/Europe/Lon don", + wantHost: "usr/share/zoneinfo/Europe/Lon don", + wantSandbox: "/usr/share/zoneinfo/localtime", + wantTZ: ":/usr/share/zoneinfo/localtime", + }, + { + name: "a link to a zone that is not there", + link: "../usr/share/zoneinfo/Europe/London", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + root := t.TempDir() + if tc.zoneFile != "" { + writeFile(t, filepath.Join(root, tc.zoneFile)) + } + + localtime := filepath.Join(root, "etc", "localtime") + require.NoError(t, os.MkdirAll(filepath.Dir(localtime), 0o755)) + if tc.link == "" { + writeFile(t, localtime) + } else { + require.NoError(t, os.Symlink(tc.link, localtime)) + } + + want := Zone{SandboxPath: tc.wantSandbox, TZ: tc.wantTZ} + if tc.wantHost != "" { + want.HostPath = resolved(t, filepath.Join(root, tc.wantHost)) + } + + assert.Equal(t, want, zoneOf(localtime)) + }) + } +} + +// A host that names a zone by one of the database's own aliases still +// shares a file every image has under the name it has there. +func TestZoneOfAnAlias(t *testing.T) { + t.Parallel() + + root := t.TempDir() + london := filepath.Join(root, "usr/share/zoneinfo/Europe/London") + writeFile(t, london) + require.NoError(t, os.Symlink("Europe/London", filepath.Join(root, "usr/share/zoneinfo/GB"))) + + localtime := filepath.Join(root, "etc", "localtime") + require.NoError(t, os.MkdirAll(filepath.Dir(localtime), 0o755)) + require.NoError(t, os.Symlink("../usr/share/zoneinfo/GB", localtime)) + + assert.Equal(t, Zone{ + HostPath: resolved(t, london), + SandboxPath: "/usr/share/zoneinfo/Europe/London", + TZ: "Europe/London", + }, zoneOf(localtime)) +} + +func TestZoneOfAHostWithoutOne(t *testing.T) { + t.Parallel() + + assert.Equal(t, Zone{}, zoneOf(filepath.Join(t.TempDir(), "etc", "localtime"))) +} + +func writeFile(t *testing.T, path string) { + t.Helper() + + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte("TZif"), 0o600)) +} + +// resolved is what the zone file's path is once the temporary directory's +// own symlinks are gone, which is what zoneOf reports. +func resolved(t *testing.T, path string) string { + t.Helper() + + out, err := filepath.EvalSymlinks(path) + require.NoError(t, err) + + return out +} diff --git a/internal/util/xkb/xkb.go b/internal/util/xkb/xkb.go index eda1cf8..4fbfcf4 100644 --- a/internal/util/xkb/xkb.go +++ b/internal/util/xkb/xkb.go @@ -45,21 +45,36 @@ func Defaults() []string { return env } - // localectl is asked first, on any session. It reports the configured - // layout, which is the one the user chose. setxkbmap reports what the - // running X server happens to hold, and the two disagree more often - // than they look like they should. - // - // Both cases were seen on real hosts. On a Wayland session setxkbmap - // asks Xwayland, which carries its own default rather than the - // compositor's keymap. On an X11 host whose localectl said gb with a - // microsoftpro model, setxkbmap still answered us and pc105, and the - // profile faithfully reproduced a layout its user does not type on. - // Preferring the configured answer is right in both. - // - // A session that really does want the live value sets XKB_DEFAULT_ - // above, which still wins over both. - return firstOf(fromLocalectl(localectlQuery), fromSetxkbmap(setxkbmapQuery)) + return preferred(os.Getenv("XDG_SESSION_TYPE"), setxkbmapQuery, localectlQuery) +} + +// preferred returns the keymap of the session, asking the two tools in the +// order that session makes reliable and falling back to the other. +// +// The two disagree more often than they look like they should, and which +// one is right depends on where it is asked. +// +// On X11 the live answer wins. setxkbmap asks the running X server, which +// is the keyboard the user is typing on, including a layout applied by +// hand after login. localectl reports what was configured, which on a host +// whose layout is set at runtime is a layout its user does not type on. +// +// On Wayland the configured answer wins. There setxkbmap reaches Xwayland, +// which carries its own default rather than the compositor's keymap, so it +// answers confidently with nobody's layout. localectl reports what the +// compositor built its keymap from. +// +// An unset session type is treated as X11. Every X11 session sets it, and +// a session that sets nothing is not a Wayland one. +// +// A host that wants neither answer sets XKB_DEFAULT_ itself, which wins +// over both. +func preferred(session string, live, configured query) []string { + if strings.EqualFold(session, "wayland") { + return firstOf(fromLocalectl(configured), fromSetxkbmap(live)) + } + + return firstOf(fromSetxkbmap(live), fromLocalectl(configured)) } func firstOf(sources ...[]string) []string { @@ -102,6 +117,10 @@ func fromEnv() []string { return nil } +// query reads a tool's output. It is what makes the two sources a seam a +// test can drive, and it is the same shape for both. +type query func() ([]byte, error) + func setxkbmapQuery() ([]byte, error) { //nolint:gosec // G204: the binary is a fixed path and the argument is a literal. return execabs.Command(files.SetxkbmapBinary, "-query").Output() @@ -125,7 +144,7 @@ var localectlFields = map[string]string{ // fromLocalectl reads the configured layout, which is what a Wayland // compositor builds its keymap from and what Xwayland does not report. -func fromLocalectl(q func() ([]byte, error)) []string { +func fromLocalectl(q query) []string { out, err := q() if err != nil { slog.Debug("cannot read the configured keyboard layout", "error", err) @@ -150,7 +169,7 @@ func fromLocalectl(q func() ([]byte, error)) []string { // fromSetxkbmap parses setxkbmap -query, which prints one "key: value" // per line and omits nothing, printing an empty value for a component // that is not set. -func fromSetxkbmap(q func() ([]byte, error)) []string { +func fromSetxkbmap(q query) []string { out, err := q() if err != nil { slog.Debug("cannot read the host keyboard layout", "error", err) diff --git a/internal/util/xkb/xkb_test.go b/internal/util/xkb/xkb_test.go index 3da9388..a669b0d 100644 --- a/internal/util/xkb/xkb_test.go +++ b/internal/util/xkb/xkb_test.go @@ -145,28 +145,51 @@ func TestFirstOfTakesTheFirstThatAnswered(t *testing.T) { require.Empty(t, firstOf(nil, nil)) } -// A real X11 host reported gb and microsoftpro from localectl while -// setxkbmap answered us and pc105. The configured layout is the one its -// user types on, so it is the one preferred, on any session. -func TestDefaultsPrefersTheConfiguredLayout(t *testing.T) { - got := firstOf( - fromLocalectl(func() ([]byte, error) { - return []byte(" X11 Layout: gb\n X11 Model: microsoftpro\n"), nil - }), - fromSetxkbmap(func() ([]byte, error) { - return []byte("layout: us\nmodel: pc105\n"), nil - }), - ) - - require.Equal(t, []string{"XKB_DEFAULT_MODEL=microsoftpro", "XKB_DEFAULT_LAYOUT=gb"}, got) +// Which of the two answers to believe depends on the session, because the +// two tools are reliable on opposite ones. On X11 setxkbmap reports the +// keymap the user is typing on, including one applied by hand after login. +// On Wayland it reports Xwayland's own default, which is nobody's layout. +func TestPreferredSource(t *testing.T) { + t.Parallel() + + const live = "rules: evdev\nmodel: pc105\nlayout: gb\n" + const configured = "X11 Layout: us\nX11 Model: pc104\n" + + tests := []struct { + name string + session string + want string + }{ + {"x11 prefers the running layout", "x11", "XKB_DEFAULT_LAYOUT=gb"}, + {"a session that says nothing is treated as x11", "", "XKB_DEFAULT_LAYOUT=gb"}, + {"wayland prefers the configured layout", "wayland", "XKB_DEFAULT_LAYOUT=us"}, + {"wayland is matched whatever its case", "Wayland", "XKB_DEFAULT_LAYOUT=us"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := preferred(tc.session, + func() ([]byte, error) { return []byte(live), nil }, + func() ([]byte, error) { return []byte(configured), nil }, + ) + + require.Contains(t, got, tc.want) + }) + } } -// A host with no localectl still gets the running layout. -func TestDefaultsFallsBackToTheRunningLayout(t *testing.T) { - got := firstOf( - fromLocalectl(func() ([]byte, error) { return nil, errors.New("not found") }), - fromSetxkbmap(func() ([]byte, error) { return []byte("layout: us\n"), nil }), - ) +// Whichever is preferred, the other still answers when the first cannot. +func TestPreferredFallsBack(t *testing.T) { + t.Parallel() + + const configured = "X11 Layout: us\n" + fails := func() ([]byte, error) { return nil, errors.New("not found") } + + got := preferred("x11", fails, func() ([]byte, error) { return []byte(configured), nil }) + require.Contains(t, got, "XKB_DEFAULT_LAYOUT=us") - require.Equal(t, []string{"XKB_DEFAULT_LAYOUT=us"}, got) + got = preferred("wayland", func() ([]byte, error) { return []byte("layout: gb\n"), nil }, fails) + require.Contains(t, got, "XKB_DEFAULT_LAYOUT=gb") }