mirror of
https://github.com/fleetdm/fleet
synced 2026-05-24 09:28:54 +00:00
This pull request addresses a startup issue with Fleet Desktop on openSUSE Leap 16 and similar Linux distributions. The main change is to adjust how Fleet Desktop and key-escrow dialogs are launched to avoid environment variable loss caused by login shell profile scripts. The fix is scoped specifically to openSUSE Leap 16+ to avoid impacting other distributions. **Distribution-specific sudo invocation changes:** * The `-i` (login shell) flag is now omitted from the `sudo` command when launching Fleet Desktop and key-escrow dialogs on openSUSE Leap 16 and newer, preventing environment variables from being lost due to profile script interference. [[1]](diffhunk://#diff-633ab361af6795ef458233819e2806dfba4ca56f684866d956321825b8fd2e91R1) [[2]](diffhunk://#diff-3e8315d9f12512bce490457c5d20bd7c5aebaa2a8e18b1abf50e504815dd7a9dR178-R193) * For all other supported distributions, the previous behavior (using `-i`) is preserved to maintain compatibility and avoid unnecessary re-testing. **Detection logic:** * Introduced a new helper function `isOpenSUSELeap16Plus` in `execuser_linux.go` to detect if the host is running openSUSE Leap 16 or newer by parsing `/etc/os-release`. This ensures the workaround is only applied where necessary. --- **Related issue:** N/A — surfaced via field investigation on openSUSE Leap 16 (arm64). This PR addresses two distinct issues that together prevent Fleet Desktop from working on openSUSE Leap 16, both validated end-to-end on a real Leap 16 (arm64) host. ## 1. Launch reliability — drop `sudo -i` `orbit/pkg/execuser/execuser_linux.go` On Linux, Orbit launches Fleet Desktop with: ``` sudo -n -i -u <user> -H env WAYLAND_DISPLAY=… … FLEET_DESKTOP_DEVICE_IDENTIFIER_PATH=/opt/orbit/identifier … /…/fleet-desktop ``` The `-i` flag makes sudo "simulate initial login" — it runs the target user's shell as a login shell and wraps the rest of the command in `bash --login -c '<escaped>'`. That sources `/etc/profile` and every script in `/etc/profile.d/*` before our `env KEY=val … fleet-desktop` line runs, and shell metacharacters (`=`, `:`, `/`, `.`) get backslash-escaped through the shell layer. On **openSUSE Leap 16 (arm64)**, that indirection causes the inline env-var assignments to not reach `fleet-desktop`, which exits immediately with: ``` FTL missing URL environment FLEET_DESKTOP_DEVICE_IDENTIFIER_PATH ``` Orbit then respawns it every ~15 s in a tight kill-and-respawn loop, so the tray icon never appears. **Fix:** drop `-i` from the sudo invocation. We don't need a login shell: - `-H` already sets `HOME` to the target user. - sudo's default `env_reset` sets `USER` / `LOGNAME` / `SHELL` / `MAIL` and `PATH` to `secure_path`. - All session vars (`WAYLAND_DISPLAY`, `DISPLAY`, `DBUS_SESSION_BUS_ADDRESS`, `LD_LIBRARY_PATH`) and every `FLEET_DESKTOP_*` var are already passed explicitly via `env KEY=val …`. After the change, sudo `execve()`s `env` directly with no shell layer in between, so `/etc/profile.d` sourcing and shell-escaping are out of the picture. The `runuser -l` /proc/keys-leak regression from PR #32309 does not apply — that was specific to `runuser -l` creating session keyrings; sudo without `-i` doesn't. # Checklist for submitter - [x] Changes file added: `orbit/changes/fleet-desktop-linux-no-login-shell` - [x] Input data is properly validated; untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops (script's wait loop now bounded at 90s). - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes — N/A. ## Testing Manual QA needed before merge: - [x] **openSUSE Leap 16 (arm64)** — Fleet Desktop process starts, stays running, env vars present, no FTL respawn loop. Done via `sudo` shim. - [x] **openSUSE Leap 16 (arm64) — extension fallback** — manual tarball install + schema compilation produces a working tray icon (matching what the script automates). - [ ] **Ubuntu 22.04 / 24.04** — regression check: Fleet Desktop tray icon still appears, key-escrow zenity dialog still renders, AppIndicator script still installs via the official path. - [ ] **Fedora (recent)** — regression check: same as above. - [ ] **Debian** — regression check: same as above. - [ ] **openSUSE Tumbleweed** — confirm `InstallRemoteExtension` path still works (no fallback path triggered). ## fleetd/orbit/Fleet Desktop - [x] Verified compatibility with the latest released version of Fleet — pure launch-flag change plus a script update; no protocol or schema impact. - [x] If the change applies to only one platform, confirmed that `runtime.GOOS` is used as needed to isolate changes — Go change is in `execuser_linux.go`, only built on Linux. The script is Linux-only by construction. - [ ] Verified that fleetd runs on macOS, Linux and Windows — Linux re-verification pending QA above; macOS/Windows code paths unchanged. - [ ] Verified auto-update works from the released version of component to the new version. ## Notes for reviewers - The tray-icon visibility issue is an OS-side prerequisite (GNOME 3.26+ has no native tray), so the AppIndicator extension is required regardless. Even after installing it, Wayland requires a logout/login to pick up new extensions — this is documented behavior and not specific to the fallback path.
336 lines
10 KiB
Go
336 lines
10 KiB
Go
package execuser
|
|
|
|
import (
|
|
"bytes"
|
|
"errors"
|
|
"fmt"
|
|
"os"
|
|
"os/exec"
|
|
"path/filepath"
|
|
"sort"
|
|
"strconv"
|
|
"strings"
|
|
"syscall"
|
|
|
|
userpkg "github.com/fleetdm/fleet/v4/orbit/pkg/user"
|
|
"github.com/rs/zerolog/log"
|
|
)
|
|
|
|
// base command to setup an exec.Cmd using `runuser`
|
|
func baserun(path string, opts eopts) (cmd *exec.Cmd, err error) {
|
|
if opts.user == "" {
|
|
return nil, errors.New("missing user")
|
|
}
|
|
|
|
args, env, err := getConfigForCommand(opts.user, path)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("get args: %w", err)
|
|
}
|
|
|
|
env = append(env,
|
|
// Append the packaged libayatana-appindicator3 libraries path to LD_LIBRARY_PATH.
|
|
//
|
|
// Fleet Desktop doesn't use libayatana-appindicator3 since 1.18.3, but we need to
|
|
// keep this to support older versions of Fleet Desktop.
|
|
fmt.Sprintf("LD_LIBRARY_PATH=%s:%s", filepath.Dir(path), os.ExpandEnv("$LD_LIBRARY_PATH")),
|
|
)
|
|
|
|
for _, nv := range opts.env {
|
|
env = append(env, fmt.Sprintf("%s=%s", nv[0], nv[1]))
|
|
}
|
|
|
|
// Hold any command line arguments to pass to the command.
|
|
cmdArgs := make([]string, 0, len(opts.args)*2)
|
|
if len(opts.args) > 0 {
|
|
for _, arg := range opts.args {
|
|
cmdArgs = append(cmdArgs, arg[0])
|
|
if arg[1] != "" {
|
|
cmdArgs = append(cmdArgs, arg[1])
|
|
}
|
|
}
|
|
}
|
|
|
|
// Run `env` to setup the environment.
|
|
args = append(args, "env")
|
|
args = append(args, env...)
|
|
// Pass the command and its arguments.
|
|
args = append(args, path)
|
|
args = append(args, cmdArgs...)
|
|
|
|
// Use sudo to run the command as the login user.
|
|
args = append([]string{"sudo"}, args...)
|
|
|
|
// If a timeout is set, prefix the command with "timeout".
|
|
if opts.timeout > 0 {
|
|
args = append([]string{"timeout", fmt.Sprintf("%ds", int(opts.timeout.Seconds()))}, args...)
|
|
}
|
|
|
|
cmd = exec.Command(args[0], args[1:]...) // #nosec G204
|
|
return
|
|
}
|
|
|
|
// run a command, passing its output to stdout and stderr.
|
|
func run(path string, opts eopts) (lastLogs string, err error) {
|
|
cmd, err := baserun(path, opts)
|
|
if err != nil {
|
|
return "", err
|
|
}
|
|
|
|
cmd.Stderr = os.Stderr
|
|
cmd.Stdout = os.Stdout
|
|
log.Info().Str("cmd", cmd.String()).Msg("running command")
|
|
|
|
if err := cmd.Start(); err != nil {
|
|
return "", fmt.Errorf("open path %q: %w", path, err)
|
|
}
|
|
return "", nil
|
|
}
|
|
|
|
// runWithOutput runs a command and return its output and exit code.
|
|
func runWithOutput(path string, opts eopts) (output []byte, exitCode int, err error) {
|
|
cmd, err := baserun(path, opts)
|
|
if err != nil {
|
|
return nil, -1, err
|
|
}
|
|
|
|
output, err = cmd.Output()
|
|
if err != nil {
|
|
if exitErr, ok := err.(*exec.ExitError); ok {
|
|
exitCode = exitErr.ExitCode()
|
|
return output, exitCode, fmt.Errorf("%q exited with code %d: %w", path, exitCode, err)
|
|
}
|
|
return output, -1, fmt.Errorf("%q error: %w", path, err)
|
|
}
|
|
|
|
return output, exitCode, nil
|
|
}
|
|
|
|
func getUserID(user string) (string, error) {
|
|
uid_, err := exec.Command("id", "-u", user).Output()
|
|
if err != nil {
|
|
return "", fmt.Errorf("failed to execute id command for %q: %w", user, err)
|
|
}
|
|
uid := strings.TrimSpace(string(uid_))
|
|
if uid == "" {
|
|
return "", errors.New("failed to get uid")
|
|
}
|
|
return uid, nil
|
|
}
|
|
|
|
func getDisplayVariableForSession(userID string, displaySessionType userpkg.GuiSessionType) string {
|
|
if displaySessionType == userpkg.GuiSessionTypeX11 {
|
|
x11Display, err := getUserX11Display(userID)
|
|
if err != nil {
|
|
log.Error().Err(err).Msg("failed to get X11 display, using default :0")
|
|
// TODO(lucas): Revisit when working on multi-user/multi-session support.
|
|
// Default to display ':0' if user display could not be found.
|
|
// This assumes there's only one desktop session and belongs to the
|
|
// user returned in `getLoginUID'.
|
|
return ":0"
|
|
}
|
|
return x11Display
|
|
}
|
|
|
|
waylandDisplay, err := getUserWaylandDisplay(userID)
|
|
if err != nil {
|
|
log.Error().Err(err).Msg("failed to get wayland display, using default wayland-0")
|
|
// TODO(lucas): Revisit when working on multi-user/multi-session support.
|
|
// Default to display 'wayland-0' if user display could not be found.
|
|
// This assumes there's only one desktop session and belongs to the
|
|
// user returned in `getLoginUID'.
|
|
return "wayland-0"
|
|
}
|
|
return waylandDisplay
|
|
}
|
|
|
|
func getConfigForCommand(user string, path string) (args []string, env []string, err error) {
|
|
// Get user ID
|
|
userID, err := getUserID(user)
|
|
if err != nil {
|
|
return nil, nil, fmt.Errorf("get user ID: %w", err)
|
|
}
|
|
log.Info().Str("user", user).Str("id", userID).Msg("attempting to get user session type and display")
|
|
|
|
// Get user's display session type.
|
|
userDisplaySession, err := userpkg.GetUserDisplaySessionType(userID)
|
|
if err != nil {
|
|
// Wayland is the default for most distributions,
|
|
// thus we assume wayland if we couldn't determine the session type.
|
|
log.Error().Err(err).Msg("assuming wayland session")
|
|
userDisplaySession = &userpkg.UserDisplaySession{
|
|
Type: userpkg.GuiSessionTypeWayland,
|
|
}
|
|
} else if userDisplaySession.Type == userpkg.GuiSessionTypeTty {
|
|
return nil, nil, fmt.Errorf("user %q (%s) is not running a GUI session", user, userID)
|
|
}
|
|
|
|
// Get user's "display" variable for the GUI session.
|
|
display := getDisplayVariableForSession(userID, userDisplaySession.Type)
|
|
|
|
log.Info().
|
|
Str("path", path).
|
|
Str("user", user).
|
|
Str("id", userID).
|
|
Str("display", display).
|
|
Str("session_type", userDisplaySession.Type.String()).
|
|
Msg("running sudo")
|
|
|
|
// On openSUSE Leap 16+ we drop -i (login shell). With -i, sudo runs the target
|
|
// user's shell as a login shell and passes the rest of the command via
|
|
// `bash --login -c`, which sources /etc/profile and /etc/profile.d/* and
|
|
// shell-escapes the inline command. On Leap 16 that environment indirection
|
|
// causes our `env KEY=val ... fleet-desktop` invocation to lose env vars, so
|
|
// fleet-desktop exits with "missing URL environment ..." and Orbit respawns it
|
|
// in a tight loop. -H sets HOME to the target user; sudo's default env_reset
|
|
// already sets USER/LOGNAME/SHELL.
|
|
//
|
|
// We keep -i on every other supported distribution to preserve the previously
|
|
// QA'd behavior.
|
|
if isOpenSUSELeap16Plus() {
|
|
args = []string{"-n", "-u", user, "-H"}
|
|
} else {
|
|
args = []string{"-n", "-i", "-u", user, "-H"}
|
|
}
|
|
env = make([]string, 0)
|
|
|
|
if userDisplaySession.Type == userpkg.GuiSessionTypeWayland {
|
|
env = append(env, "WAYLAND_DISPLAY="+display)
|
|
// For xdg-open to work on a Wayland session we still need to set the DISPLAY variable.
|
|
x11Display := ":" + strings.TrimPrefix(display, "wayland-")
|
|
env = append(env, "DISPLAY="+x11Display)
|
|
} else {
|
|
env = append(env, "DISPLAY="+display)
|
|
}
|
|
|
|
env = append(env,
|
|
// DBUS_SESSION_BUS_ADDRESS sets the location of the user login session bus.
|
|
// Required by the libayatana-appindicator3 library to display a tray icon
|
|
// on the desktop session.
|
|
//
|
|
// This is required for Ubuntu 18, and not required for Ubuntu 21/22
|
|
// (because it's already part of the user).
|
|
fmt.Sprintf("DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/%s/bus", userID),
|
|
)
|
|
|
|
return args, env, nil
|
|
}
|
|
|
|
// isOpenSUSELeap16Plus reports whether the host is running openSUSE Leap 16 or
|
|
// newer. We scope the no-login-shell sudo workaround to that distribution since
|
|
// it is the one observed to break under sudo -i; other distributions retain the
|
|
// previous (login-shell) launch path so we don't have to re-QA them.
|
|
func isOpenSUSELeap16Plus() bool {
|
|
data, err := os.ReadFile("/etc/os-release")
|
|
if err != nil {
|
|
return false
|
|
}
|
|
var id, versionID string
|
|
for line := range strings.SplitSeq(string(data), "\n") {
|
|
key, value, ok := strings.Cut(line, "=")
|
|
if !ok {
|
|
continue
|
|
}
|
|
// /etc/os-release values may be quoted.
|
|
value = strings.Trim(value, `"'`)
|
|
switch key {
|
|
case "ID":
|
|
id = value
|
|
case "VERSION_ID":
|
|
versionID = value
|
|
}
|
|
}
|
|
if id != "opensuse-leap" {
|
|
return false
|
|
}
|
|
// VERSION_ID is typically "16" or "16.0"; compare the major component.
|
|
major, _, _ := strings.Cut(versionID, ".")
|
|
n, err := strconv.Atoi(major)
|
|
if err != nil {
|
|
return false
|
|
}
|
|
return n >= 16
|
|
}
|
|
|
|
// getUserWaylandDisplay returns the value to set on WAYLAND_DISPLAY for the given user.
|
|
func getUserWaylandDisplay(uid string) (string, error) {
|
|
matches, err := filepath.Glob("/run/user/" + uid + "/wayland-*")
|
|
if err != nil {
|
|
return "", fmt.Errorf("list wayland socket files: %w", err)
|
|
}
|
|
sort.Slice(matches, func(i, j int) bool {
|
|
return matches[i] < matches[j]
|
|
})
|
|
for _, match := range matches {
|
|
if strings.HasSuffix(match, ".lock") {
|
|
continue
|
|
}
|
|
return filepath.Base(match), nil
|
|
}
|
|
return "", errors.New("wayland socket not found")
|
|
}
|
|
|
|
// getUserX11Display returns the value to set on DISPLAY for the given user.
|
|
// It scans /proc to find a process owned by the user that has DISPLAY set
|
|
// in its environment.
|
|
func getUserX11Display(userID string) (string, error) {
|
|
uid, err := strconv.ParseUint(userID, 10, 32)
|
|
if err != nil {
|
|
return "", fmt.Errorf("parse user ID %q: %w", userID, err)
|
|
}
|
|
|
|
entries, err := os.ReadDir("/proc")
|
|
if err != nil {
|
|
return "", fmt.Errorf("read /proc: %w", err)
|
|
}
|
|
|
|
for _, entry := range entries {
|
|
if !entry.IsDir() {
|
|
continue
|
|
}
|
|
// Skip non-PID directories.
|
|
if _, err := strconv.Atoi(entry.Name()); err != nil {
|
|
continue
|
|
}
|
|
// Check if the process belongs to our target user.
|
|
info, err := entry.Info()
|
|
if err != nil {
|
|
continue
|
|
}
|
|
stat, ok := info.Sys().(*syscall.Stat_t)
|
|
if !ok || stat.Uid != uint32(uid) {
|
|
continue
|
|
}
|
|
|
|
// Try to read DISPLAY from this process's environment.
|
|
display, err := readEnvFromProc(entry.Name(), "DISPLAY")
|
|
if err != nil || display == "" {
|
|
continue
|
|
}
|
|
|
|
log.Debug().Msgf("found DISPLAY variable in %q", entry.Name())
|
|
return display, nil
|
|
}
|
|
|
|
return "", fmt.Errorf("DISPLAY not found in any process for user %s", userID)
|
|
}
|
|
|
|
// readEnvFromProc reads a specific environment variable from /proc/<pid>/environ.
|
|
func readEnvFromProc(pid string, envVar string) (string, error) {
|
|
return readEnvFromProcFile(fmt.Sprintf("/proc/%s/environ", pid), envVar)
|
|
}
|
|
|
|
// readEnvFromProcFile reads a specific environment variable from a /proc environ file.
|
|
// The file contains null-byte separated KEY=VALUE entries.
|
|
func readEnvFromProcFile(path string, envVar string) (string, error) {
|
|
data, err := os.ReadFile(path)
|
|
if err != nil {
|
|
return "", err
|
|
}
|
|
prefix := envVar + "="
|
|
for entry := range bytes.SplitSeq(data, []byte{0}) {
|
|
if s := string(entry); strings.HasPrefix(s, prefix) {
|
|
return s[len(prefix):], nil
|
|
}
|
|
}
|
|
return "", nil
|
|
}
|