Skip to content
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

Add package to perform path lookups and validations for commands #1443

Merged
merged 39 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
a46d4bb
Add package to perform path lookups and validations for commands
RebeccaMahany Nov 7, 2023
dbf8320
Merge remote-tracking branch 'upstream/main' into becca/binary-paths
RebeccaMahany Nov 7, 2023
26bbbee
Add linter for use of exec.Command* functions
RebeccaMahany Nov 7, 2023
5f2e0b3
Add some missing commands
RebeccaMahany Nov 7, 2023
9f4ab8e
Merge remote-tracking branch 'upstream/main' into becca/binary-paths
RebeccaMahany Nov 7, 2023
60dfafa
Fix typo
RebeccaMahany Nov 7, 2023
bf5f04e
Update to use allowedpaths over exec
RebeccaMahany Nov 7, 2023
cd71600
Fix typos
RebeccaMahany Nov 7, 2023
c8165f6
Add more missing commands
RebeccaMahany Nov 7, 2023
baaad86
Another typo
RebeccaMahany Nov 7, 2023
5ab9450
libexec is not in path
RebeccaMahany Nov 7, 2023
292338c
More typos
RebeccaMahany Nov 7, 2023
e5dba56
New and improved
RebeccaMahany Nov 8, 2023
d244dcf
Fix comments, add test
RebeccaMahany Nov 8, 2023
7538a55
Some stragglers
RebeccaMahany Nov 8, 2023
7a35987
Merge remote-tracking branch 'upstream/main' into becca/binary-paths
RebeccaMahany Nov 8, 2023
64a1fc1
Missing context
RebeccaMahany Nov 8, 2023
4cf3358
Using CommandContext instead of Command
RebeccaMahany Nov 8, 2023
ce32ff4
Add test
RebeccaMahany Nov 8, 2023
833a5a3
Merge remote-tracking branch 'upstream/main' into becca/binary-paths
RebeccaMahany Nov 9, 2023
17de2d1
Make all commands platform-specific
RebeccaMahany Nov 9, 2023
9241dbf
Combine helper functions
RebeccaMahany Nov 9, 2023
7eca23e
Some more platform fixups
RebeccaMahany Nov 9, 2023
7328649
LookPath on NixOS only
RebeccaMahany Nov 9, 2023
3a4f2ec
Merge remote-tracking branch 'upstream/main' into becca/binary-paths
RebeccaMahany Nov 9, 2023
263a126
Fix check
RebeccaMahany Nov 9, 2023
c012de8
Renamed allowedpaths to allowedcmd
RebeccaMahany Nov 9, 2023
3772a28
Use env vars for windows paths
RebeccaMahany Nov 9, 2023
25c351e
Fix overzealous find-and-replace
RebeccaMahany Nov 9, 2023
6df84bd
cmd => cmdGen
RebeccaMahany Nov 13, 2023
0511ca8
Log and return empty results on command creation failure
RebeccaMahany Nov 13, 2023
0a3fd90
Don't unify dev table tooling commands
RebeccaMahany Nov 13, 2023
7d39c00
Fix casing for some commands
RebeccaMahany Nov 13, 2023
e4a2912
Merge remote-tracking branch 'upstream/main' into becca/binary-paths
RebeccaMahany Nov 13, 2023
6545bfe
Merge remote-tracking branch 'upstream/main' into becca/binary-paths
RebeccaMahany Nov 13, 2023
53bcf57
Make nolint:forbidigo directives more specific and add documentation
RebeccaMahany Nov 13, 2023
bef1078
Merge remote-tracking branch 'upstream/main' into becca/binary-paths
RebeccaMahany Nov 13, 2023
99b5e2a
Merge remote-tracking branch 'upstream/main' into becca/binary-paths
RebeccaMahany Nov 14, 2023
0ce1ec4
SYSTEM32 env var not available when running flare, use WINDIR instead
RebeccaMahany Nov 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ linters:
- sqlclosecheck
- unconvert
- paralleltest
- forbidigo
- sloglint
- revive
disable:
Expand All @@ -34,6 +35,10 @@ linters-settings:
ignore: github.com/go-kit/kit/log:Log
gofmt:
simplify: false
forbidigo:
forbid:
- p: ^exec\.Command.*$
msg: use pkg/allowedcmd functions instead
sloglint:
kv-only: true
context-only: true
Expand Down
16 changes: 12 additions & 4 deletions cmd/launcher/uninstall_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import (
"context"
"fmt"
"os"
"os/exec"
"strings"
"time"

"github.com/kolide/launcher/pkg/allowedcmd"
)

func removeLauncher(ctx context.Context, identifier string) error {
Expand All @@ -19,12 +20,15 @@ func removeLauncher(ctx context.Context, identifier string) error {
}

launchDaemonPList := fmt.Sprintf("/Library/LaunchDaemons/com.%s.launcher.plist", identifier)
launchCtlPath := "/bin/launchctl"
launchCtlArgs := []string{"unload", launchDaemonPList}

launchctlCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()
cmd := exec.CommandContext(launchctlCtx, launchCtlPath, launchCtlArgs...)
cmd, err := allowedcmd.Launchctl(launchctlCtx, launchCtlArgs...)
if err != nil {
fmt.Printf("could not find launchctl: %s\n", err)
return err
}
if out, err := cmd.Output(); err != nil {
fmt.Printf("error occurred while unloading launcher daemon, launchctl output %s: err: %s\n", out, err)
return err
Expand Down Expand Up @@ -55,7 +59,11 @@ func removeLauncher(ctx context.Context, identifier string) error {

pkgutiltCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()
pkgUtilcmd := exec.CommandContext(pkgutiltCtx, "/usr/sbin/pkgutil", "--forget", fmt.Sprintf("com.%s.launcher", identifier))
pkgUtilcmd, err := allowedcmd.Pkgutil(pkgutiltCtx, "--forget", fmt.Sprintf("com.%s.launcher", identifier))
if err != nil {
fmt.Printf("could not find pkgutil: %s\n", err)
return err
}

if out, err := pkgUtilcmd.Output(); err != nil {
fmt.Printf("error occurred while forgetting package: output %s: err: %s\n", out, err)
Expand Down
28 changes: 12 additions & 16 deletions cmd/launcher/uninstall_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ package main

import (
"context"
"errors"
"fmt"
"os"
"os/exec"
"strings"

"github.com/kolide/launcher/pkg/allowedcmd"
)

func removeLauncher(ctx context.Context, identifier string) error {
Expand All @@ -21,33 +23,27 @@ func removeLauncher(ctx context.Context, identifier string) error {
packageName := fmt.Sprintf("launcher-%s", identifier)

// Stop and disable launcher service
cmd := exec.CommandContext(ctx, "systemctl", []string{"disable", "--now", serviceName}...)
cmd, err := allowedcmd.Systemctl(ctx, []string{"disable", "--now", serviceName}...)
if err != nil {
fmt.Printf("could not find systemctl: %s\n", err)
return err
}
if out, err := cmd.CombinedOutput(); err != nil {
// Don't exit. Log and move on to the next uninstall command
fmt.Printf("error occurred while stopping/disabling launcher service, systemctl output %s: err: %s\n", string(out), err)
}

fileExists := func(f string) bool {
if _, err := os.Stat(f); err == nil {
return true
}
return false
}

// Tell the appropriate package manager to remove launcher
switch {
case fileExists("/usr/bin/dpkg"):
cmd = exec.CommandContext(ctx, "/usr/bin/dpkg", []string{"--purge", packageName}...)
if cmd, err := allowedcmd.Dpkg(ctx, []string{"--purge", packageName}...); err == nil {
if out, err := cmd.CombinedOutput(); err != nil {
fmt.Printf("error occurred while running dpkg --purge, output %s: err: %s\n", string(out), err)
}
case fileExists("/bin/rpm"):
cmd = exec.CommandContext(ctx, "/bin/rpm", []string{"-e", packageName}...)
} else if cmd, err := allowedcmd.Rpm(ctx, []string{"-e", packageName}...); err == nil {
if out, err := cmd.CombinedOutput(); err != nil {
fmt.Printf("error occurred while running rpm -e, output %s: err: %s\n", string(out), err)
}
default:
return fmt.Errorf("unsupported package manager")
} else {
return errors.New("unsupported package manager")
}

pathsToRemove := []string{
Expand Down
8 changes: 6 additions & 2 deletions ee/consoleuser/consoleuser_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import (
"bytes"
"context"
"fmt"
"os/exec"
"strconv"
"strings"

"github.com/kolide/launcher/pkg/allowedcmd"
)

// example scutil output
Expand Down Expand Up @@ -90,7 +91,10 @@ const (
)

func CurrentUids(ctx context.Context) ([]string, error) {
cmd := exec.CommandContext(ctx, "scutil")
cmd, err := allowedcmd.Scutil(ctx)
if err != nil {
return nil, fmt.Errorf("creating scutil command: %w", err)
}
cmd.Stdin = strings.NewReader("show State:/Users/ConsoleUser")

output, err := cmd.CombinedOutput()
Expand Down
18 changes: 13 additions & 5 deletions ee/consoleuser/consoleuser_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import (
"context"
"encoding/json"
"fmt"
"os/exec"
"strings"

"github.com/kolide/launcher/pkg/allowedcmd"
)

type listSessionsResult []struct {
Expand All @@ -18,7 +19,11 @@ type listSessionsResult []struct {
}

func CurrentUids(ctx context.Context) ([]string, error) {
output, err := exec.CommandContext(ctx, "loginctl", "list-sessions", "--no-legend", "--no-pager", "--output=json").Output()
cmd, err := allowedcmd.Loginctl(ctx, "list-sessions", "--no-legend", "--no-pager", "--output=json")
if err != nil {
return nil, fmt.Errorf("creating loginctl command: %w", err)
}
output, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("loginctl list-sessions: %w", err)
}
Expand All @@ -36,13 +41,16 @@ func CurrentUids(ctx context.Context) ([]string, error) {
continue
}

output, err := exec.CommandContext(ctx,
"loginctl",
cmd, err := allowedcmd.Loginctl(ctx,
"show-session", s.Session,
"--property=Remote",
"--property=Active",
).Output()
)
if err != nil {
return nil, fmt.Errorf("creating loginctl command: %w", err)
}

output, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("loginctl show-session (for sessionId %s): %w", s.Session, err)
}
Expand Down
2 changes: 1 addition & 1 deletion ee/desktop/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ func (r *DesktopUsersProcessesRunner) menuTemplatePath() string {

// desktopCommand invokes the launcher desktop executable with the appropriate env vars
func (r *DesktopUsersProcessesRunner) desktopCommand(executablePath, uid, socketPath, menuPath string) (*exec.Cmd, error) {
cmd := exec.Command(executablePath, "desktop")
cmd := exec.Command(executablePath, "desktop") //nolint:forbidigo // We trust that the launcher executable path is correct, so we don't need to use allowedcmd

cmd.Env = []string{
// When we set cmd.Env (as we're doing here/below), cmd will no longer include the default cmd.Environ()
Expand Down
33 changes: 30 additions & 3 deletions ee/desktop/runner/runner_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"syscall"

"github.com/go-kit/kit/log/level"
"github.com/kolide/launcher/pkg/allowedcmd"
"github.com/kolide/launcher/pkg/traces"
"github.com/shirou/gopsutil/v3/process"
)
Expand Down Expand Up @@ -90,7 +91,16 @@ func (r *DesktopUsersProcessesRunner) userEnvVars(ctx context.Context, uid strin
}

// Get the user's session so we can get their display (needed for opening notification action URLs in browser)
sessionOutput, err := exec.CommandContext(ctx, "loginctl", "show-user", uid, "--value", "--property=Sessions").Output()
cmd, err := allowedcmd.Loginctl(ctx, "show-user", uid, "--value", "--property=Sessions")
if err != nil {
level.Debug(r.logger).Log(
"msg", "could not create loginctl command",
"uid", uid,
"err", err,
)
return envVars
}
sessionOutput, err := cmd.Output()
if err != nil {
level.Debug(r.logger).Log(
"msg", "could not get user session",
Expand All @@ -108,7 +118,16 @@ func (r *DesktopUsersProcessesRunner) userEnvVars(ctx context.Context, uid strin
sessionList := strings.Split(sessions, " ")
for _, session := range sessionList {
// Figure out what type of graphical session the user has -- x11, wayland?
typeOutput, err := exec.CommandContext(ctx, "loginctl", "show-session", session, "--value", "--property=Type").Output()
cmd, err := allowedcmd.Loginctl(ctx, "show-session", session, "--value", "--property=Type")
if err != nil {
level.Debug(r.logger).Log(
"msg", "could not create loginctl command to get session type",
"uid", uid,
"err", err,
)
continue
}
typeOutput, err := cmd.Output()
if err != nil {
level.Debug(r.logger).Log(
"msg", "could not get session type",
Expand Down Expand Up @@ -147,7 +166,15 @@ func (r *DesktopUsersProcessesRunner) userEnvVars(ctx context.Context, uid strin

func (r *DesktopUsersProcessesRunner) displayFromX11(ctx context.Context, session string) string {
// We can read $DISPLAY from the session properties
xDisplayOutput, err := exec.CommandContext(ctx, "loginctl", "show-session", session, "--value", "--property=Display").Output()
cmd, err := allowedcmd.Loginctl(ctx, "show-session", session, "--value", "--property=Display")
if err != nil {
level.Debug(r.logger).Log(
"msg", "could not create command to get Display from user session",
"err", err,
)
return defaultDisplay
}
xDisplayOutput, err := cmd.Output()
if err != nil {
level.Debug(r.logger).Log(
"msg", "could not get Display from user session",
Expand Down
4 changes: 2 additions & 2 deletions ee/desktop/runner/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestDesktopUserProcessRunner_Execute(t *testing.T) {
// CPU consumption go way up.

// To get around the issue mentioned above, build the binary first and set its path as the executable path on the runner.
executablePath := filepath.Join(t.TempDir(), "desktop-test")
executablePath := filepath.Join(t.TempDir(), "desktop-test", "launcher")

if runtime.GOOS == "windows" {
executablePath = fmt.Sprintf("%s.exe", executablePath)
Expand All @@ -44,7 +44,7 @@ func TestDesktopUserProcessRunner_Execute(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

cmd := exec.CommandContext(ctx, "go", "build", "-o", executablePath, "../../../cmd/launcher")
cmd := exec.CommandContext(ctx, "go", "build", "-o", executablePath, "../../../cmd/launcher") //nolint:forbidigo // Fine to use exec.CommandContext in test
buildStartTime := time.Now()
out, err := cmd.CombinedOutput()
if err != nil {
Expand Down
12 changes: 10 additions & 2 deletions ee/desktop/user/menu/action_open_url_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,19 @@
package menu

import (
"os/exec"
"context"
"fmt"

"github.com/kolide/launcher/pkg/allowedcmd"
)

// open opens the specified URL in the default browser of the user
// See https://stackoverflow.com/a/39324149/1705598
func open(url string) error {
return exec.Command("/usr/bin/open", url).Start()
cmd, err := allowedcmd.Open(context.TODO(), url)
if err != nil {
return fmt.Errorf("creating command: %w", err)
}

return cmd.Start()
}
12 changes: 10 additions & 2 deletions ee/desktop/user/menu/action_open_url_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,19 @@
package menu

import (
"os/exec"
"context"
"fmt"

"github.com/kolide/launcher/pkg/allowedcmd"
)

// open opens the specified URL in the default browser of the user
// See https://stackoverflow.com/a/39324149/1705598
func open(url string) error {
return exec.Command("xdg-open", url).Start()
cmd, err := allowedcmd.XdgOpen(context.TODO(), url)
if err != nil {
return fmt.Errorf("creating command: %w", err)
}

return cmd.Start()
}
11 changes: 9 additions & 2 deletions ee/desktop/user/menu/action_open_url_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,21 @@
package menu

import (
"os/exec"
"context"
"fmt"
"syscall"

"github.com/kolide/launcher/pkg/allowedcmd"
)

// open opens the specified URL in the default browser of the user
// See https://stackoverflow.com/a/39324149/1705598
func open(url string) error {
cmd := exec.Command("cmd", "/C", "start", url)
cmd, err := allowedcmd.CommandPrompt(context.TODO(), "/C", "start", url)
if err != nil {
return fmt.Errorf("creating command: %w", err)
}

// https://stackoverflow.com/questions/42500570/how-to-hide-command-prompt-window-when-using-exec-in-golang
// Otherwise the cmd window will appear briefly
cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
Expand Down
Loading
Loading