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 35 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
15 changes: 12 additions & 3 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 @@ -42,7 +47,11 @@ func CurrentUids(ctx context.Context) ([]string, error) {
}

// get the active property of the session, this command does not respect the --output=json flag
output, err := exec.CommandContext(ctx, "loginctl", "show-session", s.Session, "--value", "--property=Active").Output()
cmd, err := allowedcmd.Loginctl(ctx, "show-session", s.Session, "--value", "--property=Active")
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 uid %d): %w", s.UID, err)
}
Expand Down
3 changes: 2 additions & 1 deletion ee/desktop/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,8 @@ 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")
// We trust that launcher executable path is correct, so we don't need to use allowedcmd
cmd := exec.Command(executablePath, "desktop") //nolint:forbidigo

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
3 changes: 2 additions & 1 deletion ee/desktop/runner/runner_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//nolint:forbidigo
package runner

import (
Expand Down Expand Up @@ -33,7 +34,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 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
21 changes: 11 additions & 10 deletions ee/desktop/user/notify/notify_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ package notify
import (
"context"
"fmt"
"os/exec"
"sync"
"time"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/godbus/dbus/v5"
"github.com/kolide/launcher/pkg/allowedcmd"
)

type dbusNotifier struct {
Expand All @@ -33,7 +33,7 @@ const (

// We default to xdg-open first because, if available, it appears to be better at picking
// the correct default browser.
var browserLaunchers = []string{"xdg-open", "x-www-browser"}
var browserLaunchers = []allowedcmd.AllowedCommand{allowedcmd.XdgOpen, allowedcmd.XWwwBrowser}

func NewDesktopNotifier(logger log.Logger, iconFilepath string) *dbusNotifier {
conn, err := dbus.ConnectSessionBus()
Expand Down Expand Up @@ -89,7 +89,11 @@ func (d *dbusNotifier) Listen() error {
for _, browserLauncher := range browserLaunchers {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
defer cancel()
cmd := exec.CommandContext(ctx, browserLauncher, actionUri)
cmd, err := browserLauncher(ctx, actionUri)
if err != nil {
level.Warn(d.logger).Log("msg", "couldn't create command to start process", "err", err, "browser_launcher", browserLauncher)
continue
}
if err := cmd.Start(); err != nil {
level.Error(d.logger).Log("msg", "couldn't start process", "err", err, "browser_launcher", browserLauncher)
} else {
Expand Down Expand Up @@ -166,12 +170,6 @@ func (d *dbusNotifier) sendNotificationViaDbus(n Notification) error {
}

func (d *dbusNotifier) sendNotificationViaNotifySend(n Notification) error {
notifySend, err := exec.LookPath("notify-send")
if err != nil {
level.Debug(d.logger).Log("msg", "notify-send not installed", "err", err)
return fmt.Errorf("notify-send not installed: %w", err)
}

// notify-send doesn't support actions, but URLs in notifications are clickable in at least
// some desktop environments.
if n.ActionUri != "" {
Expand All @@ -183,7 +181,10 @@ func (d *dbusNotifier) sendNotificationViaNotifySend(n Notification) error {
args = append(args, "-i", d.iconFilepath)
}

cmd := exec.Command(notifySend, args...)
cmd, err := allowedcmd.NotifySend(context.TODO(), args...)
if err != nil {
return fmt.Errorf("creating command: %w", err)
}
if out, err := cmd.CombinedOutput(); err != nil {
level.Error(d.logger).Log("msg", "could not send notification via notify-send", "output", string(out), "err", err)
return fmt.Errorf("could not send notification via notify-send: %s: %w", string(out), err)
Expand Down
Loading
Loading