-
Notifications
You must be signed in to change notification settings - Fork 111
init ee/currentprocess, centralize current process query APIs, fix unprivileged desktop runner #2804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
init ee/currentprocess, centralize current process query APIs, fix unprivileged desktop runner #2804
Changes from all commits
beceab6
51b74c1
6116efb
2173d8c
4b82a10
6b48b14
a04cd28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| // Package currentprocess provides information on the running process. | ||
| package currentprocess |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| //go:build !windows | ||
|
|
||
| package currentprocess | ||
|
|
||
| import ( | ||
| "os" | ||
| "strconv" | ||
| ) | ||
|
|
||
| // Returns whether the current process is root. | ||
| func IsElevated() (bool, error) { | ||
| return os.Geteuid() == 0, nil | ||
| } | ||
|
|
||
| // Returns the current process's numerical user id. All platforms | ||
| // return strings because of Windows. | ||
| func Uid() (string, error) { | ||
| return strconv.Itoa(os.Getuid()), nil | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| package currentprocess | ||
|
|
||
| import ( | ||
| "os/user" | ||
| "runtime" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| "go.uber.org/goleak" | ||
| ) | ||
|
|
||
| func TestMain(m *testing.M) { | ||
| goleak.VerifyTestMain(m) | ||
| } | ||
|
|
||
| func TestIsElevated(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| // Cannot consistently assert outcome, but it should never error. | ||
| _, err := IsElevated() | ||
| require.NoError(t, err) | ||
| } | ||
|
|
||
| func TestUid(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| currentUser, err := user.Current() | ||
| require.NoError(t, err) | ||
|
|
||
| expected := currentUser.Uid | ||
| if runtime.GOOS == "windows" { | ||
| expected = currentUser.Username | ||
| } | ||
|
|
||
| uid, err := Uid() | ||
| require.NoError(t, err) | ||
| require.Equal(t, expected, uid) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ import ( | |
| "github.com/kolide/launcher/v2/ee/agent/types" | ||
| "github.com/kolide/launcher/v2/ee/allowedcmd" | ||
| "github.com/kolide/launcher/v2/ee/consoleuser" | ||
| "github.com/kolide/launcher/v2/ee/currentprocess" | ||
| runnerserver "github.com/kolide/launcher/v2/ee/desktop/runner/server" | ||
| "github.com/kolide/launcher/v2/ee/desktop/user/client" | ||
| "github.com/kolide/launcher/v2/ee/desktop/user/menu" | ||
|
|
@@ -112,6 +113,11 @@ func (e NoExplorerProcessError) Error() string { | |
| return fmt.Sprintf("no explorer process found for uid: %s", e.uid) | ||
| } | ||
|
|
||
| func (e NoExplorerProcessError) Is(target error) bool { | ||
| _, ok := target.(NoExplorerProcessError) | ||
| return ok | ||
| } | ||
|
|
||
| // DesktopUsersProcessesRunner creates a launcher desktop process each time it detects | ||
| // a new console (GUI) user. If the current console user's desktop process dies, it | ||
| // will create a new one. | ||
|
|
@@ -146,8 +152,12 @@ type DesktopUsersProcessesRunner struct { | |
| knapsack types.Knapsack | ||
| // runnerServer is a local server that desktop processes call to monitor parent | ||
| runnerServer *runnerserver.RunnerServer | ||
| // osVersion is the version of the OS cached in new | ||
| // osVersion is the version of the OS, cached in new | ||
| osVersion string | ||
| // currentUid is the process owning uid, cached in new | ||
| currentUid string | ||
| // elevated is whether the process runs elevated, cached in new | ||
| elevated bool | ||
| // cachedMenuData is the cached label values of the currently displayed menu data, used for detecting changes | ||
| cachedMenuData *menuItemCache | ||
| } | ||
|
|
@@ -190,6 +200,25 @@ func New(k types.Knapsack, messenger runnerserver.Messenger, opts ...desktopUser | |
|
|
||
| runner.slogger = k.Slogger().With("component", "desktop_runner") | ||
|
|
||
| elevated, err := currentprocess.IsElevated() | ||
| if err != nil { | ||
| runner.slogger.Log(context.TODO(), slog.LevelWarn, | ||
| "failed to check if process is elevated, will assume process is privileged", | ||
| "err", err, | ||
| ) | ||
| elevated = true // fail loud: maybe succeed rather than never try | ||
| } | ||
| runner.elevated = elevated | ||
|
|
||
| runner.currentUid, err = currentprocess.Uid() | ||
| if err != nil { | ||
| runner.slogger.Log(context.TODO(), slog.LevelWarn, | ||
| "failed to get current process uid, will behave like system process if privileged", | ||
| "elevated", elevated, | ||
| "err", err, | ||
| ) | ||
| } | ||
|
|
||
| for _, opt := range opts { | ||
| opt(runner) | ||
| } | ||
|
|
@@ -843,6 +872,14 @@ func (r *DesktopUsersProcessesRunner) writeDefaultMenuTemplateFile() { | |
| } | ||
| } | ||
|
|
||
| // isCurrentUser reports whether uid is the user this process runs as. Windows UIDs | ||
| // are fully qualified names, hence the case insensitive check. | ||
| func (r *DesktopUsersProcessesRunner) isCurrentUser(uid string) bool { | ||
| return strings.EqualFold(uid, r.currentUid) | ||
| } | ||
|
|
||
| // Fans out over all users on this device. We can spawn a launcher desktop subprocess | ||
| // for console users if required and we have permission. At minimum, we check for ourselves. | ||
| func (r *DesktopUsersProcessesRunner) runConsoleUserDesktop() error { | ||
| if r.knapsack.InModernStandby() { | ||
| r.slogger.Log(context.TODO(), slog.LevelDebug, | ||
|
|
@@ -857,16 +894,30 @@ func (r *DesktopUsersProcessesRunner) runConsoleUserDesktop() error { | |
| ctx, cancel := context.WithTimeout(context.Background(), 40*time.Second) | ||
| defer cancel() | ||
|
|
||
| consoleUsers, err := consoleuser.CurrentUids(ctx) | ||
| if err != nil { | ||
| return fmt.Errorf("getting console users: %w", err) | ||
| var consoleUsers []string | ||
|
|
||
| // Querying console users on Windows requires privilege, other platforms | ||
| // use session information to establish process configuration even when ran | ||
| // unprivileged | ||
| if runtime.GOOS == "windows" && !r.elevated { | ||
| consoleUsers = []string{r.currentUid} | ||
|
Comment on lines
+899
to
+903
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love this, but an alternative distracts from my changeset: extract the loop's body below and call it for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am fine with this -- since the launcher parent process being unprivileged is a new case (i.e. we always currently expect the parent process to be privileged), we can define whatever behavior we want here. |
||
| } else { | ||
| var err error | ||
| if consoleUsers, err = consoleuser.CurrentUids(ctx); err != nil { | ||
| return fmt.Errorf("getting console users: %w", err) | ||
| } | ||
| } | ||
|
|
||
| for _, uid := range consoleUsers { | ||
| if r.userHasDesktopProcess(uid) { | ||
| continue | ||
| } | ||
|
|
||
| // Only a privileged process can start a process for somebody else. | ||
| if !r.isCurrentUser(uid) && !r.elevated { | ||
| continue | ||
| } | ||
|
|
||
| // Check to see if necessary dependencies are running on macOS before we spawn the desktop process. | ||
| // This will block for up to 30 seconds, at which point we proceed with trying to spawn anyway. | ||
| r.waitForReadyToSpawnDesktopState(ctx, uid) | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The root check was harmful in some cases. When ran as a systemd user unit (which seems plausible) or tmux (which is probably just me), the process won't have DISPLAY. Also worth noting that |
Uh oh!
There was an error while loading. Please reload this page.