From c4e5d9fc9540f1145f774a7bdcaf9fac3305e345 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Fri, 26 Apr 2024 14:48:43 -0700 Subject: [PATCH 01/14] touchid system config table use table helpers exec --- pkg/osquery/table/touchid_system_darwin.go | 28 +++++++--------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/pkg/osquery/table/touchid_system_darwin.go b/pkg/osquery/table/touchid_system_darwin.go index 483c4f824..4f418c215 100644 --- a/pkg/osquery/table/touchid_system_darwin.go +++ b/pkg/osquery/table/touchid_system_darwin.go @@ -1,15 +1,14 @@ package table import ( - "bytes" "context" - "fmt" "log/slog" "regexp" "strings" "time" "github.com/kolide/launcher/ee/allowedcmd" + "github.com/kolide/launcher/ee/tables/tablehelpers" "github.com/osquery/osquery-go/plugin/table" ) @@ -39,23 +38,18 @@ func (t *touchIDSystemConfigTable) generate(ctx context.Context, queryContext ta var results []map[string]string var touchIDCompatible, secureEnclaveCPU, touchIDEnabled, touchIDUnlock string - // Read the security chip from system_profiler - var stdout bytes.Buffer - cmd, err := allowedcmd.SystemProfiler(ctx, "SPiBridgeDataType") + stdout, err := tablehelpers.Exec(ctx, t.slogger, 15, allowedcmd.SystemProfiler, []string{"SPiBridgeDataType"}, false) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, - "could not create system_profiler command", + "execing system_profiler SPiBridgeDataType", "err", err, ) return results, nil } - cmd.Stdout = &stdout - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("calling system_profiler: %w", err) - } + // Read the security chip from system_profiler r := regexp.MustCompile(` (?PT\d) `) // Matching on: Apple T[1|2] Security Chip - match := r.FindStringSubmatch(stdout.String()) + match := r.FindStringSubmatch(string(stdout)) if len(match) == 0 { secureEnclaveCPU = "" } else { @@ -63,20 +57,16 @@ func (t *touchIDSystemConfigTable) generate(ctx context.Context, queryContext ta } // Read the system's bioutil configuration - stdout.Reset() - cmd, err = allowedcmd.Bioutil(ctx, "-r", "-s") + stdout, err = tablehelpers.Exec(ctx, t.slogger, 15, allowedcmd.Bioutil, []string{"-r", "-s"}, false) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, - "could not create bioutil command", + "execing bioutil", "err", err, ) return results, nil } - cmd.Stdout = &stdout - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("calling bioutil for system configuration: %w", err) - } - configOutStr := stdout.String() + + configOutStr := string(stdout) configSplit := strings.Split(configOutStr, ":") if len(configSplit) >= 3 { touchIDCompatible = "1" From 2d81bd53e0f3283d588d8f095d343fa7b12236d7 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Tue, 30 Apr 2024 14:49:42 -0700 Subject: [PATCH 02/14] touchid system table use tablehelper exec --- pkg/osquery/table/touchid_system_darwin.go | 4 -- pkg/osquery/table/touchid_user_darwin.go | 58 ++++------------------ 2 files changed, 11 insertions(+), 51 deletions(-) diff --git a/pkg/osquery/table/touchid_system_darwin.go b/pkg/osquery/table/touchid_system_darwin.go index 4f418c215..e346768e3 100644 --- a/pkg/osquery/table/touchid_system_darwin.go +++ b/pkg/osquery/table/touchid_system_darwin.go @@ -5,7 +5,6 @@ import ( "log/slog" "regexp" "strings" - "time" "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/tables/tablehelpers" @@ -32,9 +31,6 @@ type touchIDSystemConfigTable struct { // TouchIDSystemConfigGenerate will be called whenever the table is queried. func (t *touchIDSystemConfigTable) generate(ctx context.Context, queryContext table.QueryContext) ([]map[string]string, error) { - ctx, cancel := context.WithTimeout(ctx, 10*time.Second) - defer cancel() - var results []map[string]string var touchIDCompatible, secureEnclaveCPU, touchIDEnabled, touchIDUnlock string diff --git a/pkg/osquery/table/touchid_user_darwin.go b/pkg/osquery/table/touchid_user_darwin.go index a6c2fd887..87f493248 100644 --- a/pkg/osquery/table/touchid_user_darwin.go +++ b/pkg/osquery/table/touchid_user_darwin.go @@ -1,18 +1,14 @@ package table import ( - "bytes" "context" "errors" - "fmt" "log/slog" "os/user" - "strconv" "strings" - "syscall" - "time" "github.com/kolide/launcher/ee/allowedcmd" + "github.com/kolide/launcher/ee/tables/tablehelpers" "github.com/osquery/osquery-go/plugin/table" ) @@ -50,7 +46,7 @@ func (t *touchIDUserConfigTable) generate(ctx context.Context, queryContext tabl var touchIDUnlock, touchIDApplePay, effectiveUnlock, effectiveApplePay string // Verify the user exists on the system before proceeding - _, err := user.LookupId(constraint.Expression) + u, err := user.LookupId(constraint.Expression) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "nonexistent user", @@ -59,19 +55,18 @@ func (t *touchIDUserConfigTable) generate(ctx context.Context, queryContext tabl ) continue } - uid, _ := strconv.Atoi(constraint.Expression) // Get the user's TouchID config - configOutput, err := runCommandContext(ctx, uid, allowedcmd.Bioutil, "-r") + configOutput, err := tablehelpers.Exec(ctx, t.slogger, 15, allowedcmd.Bioutil, []string{"-r"}, false, tablehelpers.WithUid(u.Uid)) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "could not run bioutil -r", - "uid", uid, + "uid", u.Uid, "err", err, ) continue } - configSplit := strings.Split(configOutput, ":") + configSplit := strings.Split(string(configOutput), ":") // If the length of the split is 2, TouchID is not configured for this user // Otherwise, extract the values from the split. @@ -85,23 +80,23 @@ func (t *touchIDUserConfigTable) generate(ctx context.Context, queryContext tabl } else { t.slogger.Log(ctx, slog.LevelDebug, "bioutil -r returned unexpected output", - "uid", uid, - "output", configOutput, + "uid", u.Uid, + "output", string(configOutput), ) continue } // Grab the fingerprint count - countOutStr, err := runCommandContext(ctx, uid, allowedcmd.Bioutil, "-c") + countOut, err := tablehelpers.Exec(ctx, t.slogger, 15, allowedcmd.Bioutil, []string{"-c"}, false, tablehelpers.WithUid(u.Uid)) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "could not run bioutil -c", - "uid", uid, + "uid", u.Uid, "err", err, ) continue } - countSplit := strings.Split(countOutStr, ":") + countSplit := strings.Split(string(countOut), ":") fingerprintCount := strings.ReplaceAll(countSplit[1], "\t", "")[:1] // If the fingerprint count is 0, set effective values to 0 @@ -112,7 +107,7 @@ func (t *touchIDUserConfigTable) generate(ctx context.Context, queryContext tabl } result := map[string]string{ - "uid": strconv.Itoa(uid), + "uid": u.Uid, "fingerprints_registered": fingerprintCount, "touchid_unlock": touchIDUnlock, "touchid_applepay": touchIDApplePay, @@ -124,34 +119,3 @@ func (t *touchIDUserConfigTable) generate(ctx context.Context, queryContext tabl return results, nil } - -// runCommand runs a given command and arguments as the supplied user -func runCommandContext(ctx context.Context, uid int, cmd allowedcmd.AllowedCommand, args ...string) (string, error) { - ctx, cancel := context.WithTimeout(ctx, 10*time.Second) - defer cancel() - - // Set up the command - var stdout bytes.Buffer - c, err := cmd(ctx, args...) - if err != nil { - return "", fmt.Errorf("creating command: %w", err) - } - c.Stdout = &stdout - - // Check if the supplied UID is that of the current user - currentUser, err := user.Current() - if err != nil { - return "", err - } - if strconv.Itoa(uid) != currentUser.Uid { - c.SysProcAttr = &syscall.SysProcAttr{} - c.SysProcAttr.Credential = &syscall.Credential{Uid: uint32(uid), Gid: 20} - } - - // Run the command - if err := c.Run(); err != nil { - return "", err - } - - return stdout.String(), nil -} From 7e8d3e7b66ef284abc791313bd339b0907962496 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Wed, 1 May 2024 14:38:08 -0700 Subject: [PATCH 03/14] exec helper withdir option, dism use exec helper --- .../dism_default_associations.go} | 34 ++++++------------- ee/tables/tablehelpers/exec.go | 7 ++++ pkg/osquery/table/touchid_system_darwin.go | 4 +-- pkg/osquery/table/touchid_user_darwin.go | 4 +-- 4 files changed, 21 insertions(+), 28 deletions(-) rename ee/tables/{dsim_default_associations/dsim_default_associations.go => dism_default_associations/dism_default_associations.go} (77%) diff --git a/ee/tables/dsim_default_associations/dsim_default_associations.go b/ee/tables/dism_default_associations/dism_default_associations.go similarity index 77% rename from ee/tables/dsim_default_associations/dsim_default_associations.go rename to ee/tables/dism_default_associations/dism_default_associations.go index ae0527190..c2eb7439d 100644 --- a/ee/tables/dsim_default_associations/dsim_default_associations.go +++ b/ee/tables/dism_default_associations/dism_default_associations.go @@ -4,14 +4,12 @@ package dsim_default_associations import ( - "bytes" "context" "fmt" "log/slog" "os" "path/filepath" "strings" - "time" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/allowedcmd" @@ -77,36 +75,24 @@ func (t *Table) execDism(ctx context.Context) ([]byte, error) { return nil, fmt.Errorf("creating kolide_dism tmp dir: %w", err) } defer os.RemoveAll(dir) - - dstFile := "associations.xml" - var stdout bytes.Buffer - var stderr bytes.Buffer - - ctx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - + const dstFile = "associations.xml" args := []string{"/online", "/Export-DefaultAppAssociations:" + dstFile} - cmd, err := allowedcmd.Dism(ctx, args...) + out, err := tablehelpers.Exec(ctx, t.slogger, 30, allowedcmd.Dism, args, true, tablehelpers.WithDir(dir)) if err != nil { - return nil, fmt.Errorf("creating command: %w", err) - } - cmd.Dir = dir - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - t.slogger.Log(ctx, slog.LevelDebug, - "calling dsim", - "args", cmd.Args, - ) + t.slogger.Log(ctx, slog.LevelDebug, + "execing dism", + "out", string(out), + "err", err, + "args", args, + ) - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("calling dism. Got: %s: %w", stderr.String(), err) + return nil, fmt.Errorf("execing dism: %w", err) } data, err := os.ReadFile(filepath.Join(dir, dstFile)) if err != nil { - return nil, fmt.Errorf("error reading dism output file: %s: %w", err, err) + return nil, fmt.Errorf("reading dism output file: %w", err) } return data, nil diff --git a/ee/tables/tablehelpers/exec.go b/ee/tables/tablehelpers/exec.go index f8392052c..50dcfba98 100644 --- a/ee/tables/tablehelpers/exec.go +++ b/ee/tables/tablehelpers/exec.go @@ -19,6 +19,13 @@ import ( // An example of this is to run the exec as a specific user instead of root. type ExecOps func(*exec.Cmd) error +func WithDir(dir string) ExecOps { + return func(cmd *exec.Cmd) error { + cmd.Dir = dir + return nil + } +} + // Exec is a wrapper over exec.CommandContext. It does a couple of // additional things to help with table usage: // 1. It enforces a timeout. diff --git a/pkg/osquery/table/touchid_system_darwin.go b/pkg/osquery/table/touchid_system_darwin.go index e346768e3..9a3549e8f 100644 --- a/pkg/osquery/table/touchid_system_darwin.go +++ b/pkg/osquery/table/touchid_system_darwin.go @@ -34,7 +34,7 @@ func (t *touchIDSystemConfigTable) generate(ctx context.Context, queryContext ta var results []map[string]string var touchIDCompatible, secureEnclaveCPU, touchIDEnabled, touchIDUnlock string - stdout, err := tablehelpers.Exec(ctx, t.slogger, 15, allowedcmd.SystemProfiler, []string{"SPiBridgeDataType"}, false) + stdout, err := tablehelpers.Exec(ctx, t.slogger, 10, allowedcmd.SystemProfiler, []string{"SPiBridgeDataType"}, false) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "execing system_profiler SPiBridgeDataType", @@ -53,7 +53,7 @@ func (t *touchIDSystemConfigTable) generate(ctx context.Context, queryContext ta } // Read the system's bioutil configuration - stdout, err = tablehelpers.Exec(ctx, t.slogger, 15, allowedcmd.Bioutil, []string{"-r", "-s"}, false) + stdout, err = tablehelpers.Exec(ctx, t.slogger, 10, allowedcmd.Bioutil, []string{"-r", "-s"}, false) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "execing bioutil", diff --git a/pkg/osquery/table/touchid_user_darwin.go b/pkg/osquery/table/touchid_user_darwin.go index 87f493248..639963a4e 100644 --- a/pkg/osquery/table/touchid_user_darwin.go +++ b/pkg/osquery/table/touchid_user_darwin.go @@ -57,7 +57,7 @@ func (t *touchIDUserConfigTable) generate(ctx context.Context, queryContext tabl } // Get the user's TouchID config - configOutput, err := tablehelpers.Exec(ctx, t.slogger, 15, allowedcmd.Bioutil, []string{"-r"}, false, tablehelpers.WithUid(u.Uid)) + configOutput, err := tablehelpers.Exec(ctx, t.slogger, 10, allowedcmd.Bioutil, []string{"-r"}, false, tablehelpers.WithUid(u.Uid)) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "could not run bioutil -r", @@ -87,7 +87,7 @@ func (t *touchIDUserConfigTable) generate(ctx context.Context, queryContext tabl } // Grab the fingerprint count - countOut, err := tablehelpers.Exec(ctx, t.slogger, 15, allowedcmd.Bioutil, []string{"-c"}, false, tablehelpers.WithUid(u.Uid)) + countOut, err := tablehelpers.Exec(ctx, t.slogger, 10, allowedcmd.Bioutil, []string{"-c"}, false, tablehelpers.WithUid(u.Uid)) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "could not run bioutil -c", From c44eac75933aa21b7e20ef633191b524dc5bfc9a Mon Sep 17 00:00:00 2001 From: James Pickett Date: Wed, 1 May 2024 15:23:28 -0700 Subject: [PATCH 04/14] fix dism naming --- .../dism_default_associations/dism_default_associations.go | 2 +- pkg/osquery/table/platform_tables_windows.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/tables/dism_default_associations/dism_default_associations.go b/ee/tables/dism_default_associations/dism_default_associations.go index c2eb7439d..a73baa26b 100644 --- a/ee/tables/dism_default_associations/dism_default_associations.go +++ b/ee/tables/dism_default_associations/dism_default_associations.go @@ -1,7 +1,7 @@ //go:build windows // +build windows -package dsim_default_associations +package dism_default_associations import ( "context" diff --git a/pkg/osquery/table/platform_tables_windows.go b/pkg/osquery/table/platform_tables_windows.go index f41e336e4..2ee6f8287 100644 --- a/pkg/osquery/table/platform_tables_windows.go +++ b/pkg/osquery/table/platform_tables_windows.go @@ -8,7 +8,7 @@ import ( "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/tables/dataflattentable" - "github.com/kolide/launcher/ee/tables/dsim_default_associations" + "github.com/kolide/launcher/ee/tables/dism_default_associations" "github.com/kolide/launcher/ee/tables/execparsers/dsregcmd" "github.com/kolide/launcher/ee/tables/secedit" "github.com/kolide/launcher/ee/tables/wifi_networks" @@ -20,7 +20,7 @@ import ( func platformSpecificTables(slogger *slog.Logger, currentOsquerydBinaryPath string) []osquery.OsqueryPlugin { return []osquery.OsqueryPlugin{ ProgramIcons(), - dsim_default_associations.TablePlugin(slogger), + dism_default_associations.TablePlugin(slogger), secedit.TablePlugin(slogger), wifi_networks.TablePlugin(slogger), windowsupdatetable.TablePlugin(windowsupdatetable.UpdatesTable, slogger), From 7855a6e60bf844892a1650e9ff956557ecf66974 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 2 May 2024 15:23:51 -0700 Subject: [PATCH 05/14] tablehelpers refactor to use Run and RunSimple --- ee/tables/airport/table_darwin.go | 2 +- .../apple_silicon_security_policy/table.go | 2 +- ee/tables/crowdstrike/falconctl/table.go | 6 +- ee/tables/crowdstrike/falconctl/table_test.go | 2 +- ee/tables/dataflattentable/exec.go | 2 +- ee/tables/dataflattentable/exec_and_parse.go | 13 +- ee/tables/dev_table_tooling/table.go | 2 +- .../dism_default_associations.go | 4 +- ee/tables/filevault/filevault.go | 2 +- ee/tables/firmwarepasswd/firmwarepasswd.go | 43 +++---- ee/tables/gsettings/gsettings.go | 58 ++------- ee/tables/gsettings/gsettings_test.go | 3 +- ee/tables/homebrew/upgradeable.go | 15 ++- ee/tables/ioreg/ioreg.go | 2 +- ee/tables/mdmclient/mdmclient.go | 2 +- ee/tables/profiles/profiles.go | 2 +- ee/tables/spotlight/spotlight.go | 2 +- ee/tables/tablehelpers/exec.go | 61 +++++----- ee/tables/tablehelpers/exec_test.go | 114 ++++++++++++++---- ee/tables/zfs/tables.go | 2 +- pkg/osquery/table/touchid_system_darwin.go | 4 +- pkg/osquery/table/touchid_user_darwin.go | 4 +- 22 files changed, 193 insertions(+), 154 deletions(-) diff --git a/ee/tables/airport/table_darwin.go b/ee/tables/airport/table_darwin.go index 4c0bb6890..1e43d6f12 100644 --- a/ee/tables/airport/table_darwin.go +++ b/ee/tables/airport/table_darwin.go @@ -49,7 +49,7 @@ type airportExecutor struct { } func (a *airportExecutor) Exec(option string) ([]byte, error) { - return tablehelpers.Exec(a.ctx, a.slogger, 30, allowedcmd.Airport, []string{"--" + option}, false) + return tablehelpers.RunSimple(a.ctx, a.slogger, 30, allowedcmd.Airport, []string{"--" + option}) } type executor interface { diff --git a/ee/tables/apple_silicon_security_policy/table.go b/ee/tables/apple_silicon_security_policy/table.go index 386ebae9d..278a038b1 100644 --- a/ee/tables/apple_silicon_security_policy/table.go +++ b/ee/tables/apple_silicon_security_policy/table.go @@ -37,7 +37,7 @@ func TablePlugin(slogger *slog.Logger) *table.Plugin { func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ([]map[string]string, error) { var results []map[string]string - output, err := tablehelpers.Exec(ctx, t.slogger, 30, allowedcmd.Bputil, []string{bootPolicyUtilArgs}, false) + output, err := tablehelpers.RunSimple(ctx, t.slogger, 30, allowedcmd.Bputil, []string{bootPolicyUtilArgs}) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "bputil failed", diff --git a/ee/tables/crowdstrike/falconctl/table.go b/ee/tables/crowdstrike/falconctl/table.go index 4442b5c0d..cb9c07b76 100644 --- a/ee/tables/crowdstrike/falconctl/table.go +++ b/ee/tables/crowdstrike/falconctl/table.go @@ -38,7 +38,7 @@ var ( defaultOption = strings.Join(allowedOptions, " ") ) -type execFunc func(context.Context, *slog.Logger, int, allowedcmd.AllowedCommand, []string, bool, ...tablehelpers.ExecOps) ([]byte, error) +type execFunc func(context.Context, *slog.Logger, int, allowedcmd.AllowedCommand, []string, ...tablehelpers.ExecOps) ([]byte, error) type falconctlOptionsTable struct { slogger *slog.Logger @@ -54,7 +54,7 @@ func NewFalconctlOptionTable(slogger *slog.Logger) *table.Plugin { t := &falconctlOptionsTable{ slogger: slogger.With("table", "kolide_falconctl_options"), tableName: "kolide_falconctl_options", - execFunc: tablehelpers.Exec, + execFunc: tablehelpers.RunSimple, } return table.NewPlugin(t.tableName, columns, t.generate) @@ -92,7 +92,7 @@ OUTER: // then the list of options to fetch. Set the command line thusly. args := append([]string{"-g"}, options...) - output, err := t.execFunc(ctx, t.slogger, 30, allowedcmd.Falconctl, args, false) + output, err := t.execFunc(ctx, t.slogger, 30, allowedcmd.Falconctl, args) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "exec failed", diff --git a/ee/tables/crowdstrike/falconctl/table_test.go b/ee/tables/crowdstrike/falconctl/table_test.go index 6a443aebb..48d1b7fdc 100644 --- a/ee/tables/crowdstrike/falconctl/table_test.go +++ b/ee/tables/crowdstrike/falconctl/table_test.go @@ -86,7 +86,7 @@ func TestOptionRestrictions(t *testing.T) { } } -func noopExec(ctx context.Context, slogger *slog.Logger, _ int, _ allowedcmd.AllowedCommand, args []string, _ bool, _ ...tablehelpers.ExecOps) ([]byte, error) { +func noopExec(ctx context.Context, slogger *slog.Logger, _ int, _ allowedcmd.AllowedCommand, args []string, _ ...tablehelpers.ExecOps) ([]byte, error) { slogger.Log(ctx, slog.LevelInfo, "exec-in-test", "args", strings.Join(args, " ")) return []byte{}, nil } diff --git a/ee/tables/dataflattentable/exec.go b/ee/tables/dataflattentable/exec.go index c726419d8..fbae4c70f 100644 --- a/ee/tables/dataflattentable/exec.go +++ b/ee/tables/dataflattentable/exec.go @@ -47,7 +47,7 @@ func TablePluginExec(slogger *slog.Logger, tableName string, dataSourceType Data func (t *Table) generateExec(ctx context.Context, queryContext table.QueryContext) ([]map[string]string, error) { var results []map[string]string - execBytes, err := tablehelpers.Exec(ctx, t.slogger, 50, t.cmdGen, t.execArgs, false) + execBytes, err := tablehelpers.RunSimple(ctx, t.slogger, 50, t.cmdGen, t.execArgs) if err != nil { // exec will error if there's no binary, so we never want to record that if os.IsNotExist(errors.Cause(err)) { diff --git a/ee/tables/dataflattentable/exec_and_parse.go b/ee/tables/dataflattentable/exec_and_parse.go index 6d87fd603..7559f7a31 100644 --- a/ee/tables/dataflattentable/exec_and_parse.go +++ b/ee/tables/dataflattentable/exec_and_parse.go @@ -1,7 +1,9 @@ package dataflattentable import ( + "bytes" "context" + "io" "log/slog" "os" "strings" @@ -72,9 +74,14 @@ func (t *execTableV2) generate(ctx context.Context, queryContext table.QueryCont defer span.End() var results []map[string]string + var stdout bytes.Buffer + stdErr := io.Discard - execOutput, err := tablehelpers.Exec(ctx, t.slogger, t.timeoutSeconds, t.cmd, t.execArgs, t.includeStderr) - if err != nil { + if t.includeStderr { + stdErr = &stdout + } + + if err := tablehelpers.Run(ctx, t.slogger, t.timeoutSeconds, t.cmd, t.execArgs, &stdout, stdErr); err != nil { // exec will error if there's no binary, so we never want to record that if os.IsNotExist(errors.Cause(err)) { return nil, nil @@ -96,7 +103,7 @@ func (t *execTableV2) generate(ctx context.Context, queryContext table.QueryCont flattenOpts = append(flattenOpts, dataflatten.WithDebugLogging()) } - flattened, err := t.flattener.FlattenBytes(execOutput, flattenOpts...) + flattened, err := t.flattener.FlattenBytes(stdout.Bytes(), flattenOpts...) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "failure flattening output", diff --git a/ee/tables/dev_table_tooling/table.go b/ee/tables/dev_table_tooling/table.go index 4e468b641..02f8998b3 100644 --- a/ee/tables/dev_table_tooling/table.go +++ b/ee/tables/dev_table_tooling/table.go @@ -67,7 +67,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( "error": "", } - if output, err := tablehelpers.Exec(ctx, t.slogger, 30, cmd.bin, cmd.args, false); err != nil { + if output, err := tablehelpers.RunSimple(ctx, t.slogger, 30, cmd.bin, cmd.args); err != nil { t.slogger.Log(ctx, slog.LevelInfo, "execution failed", "name", name, diff --git a/ee/tables/dism_default_associations/dism_default_associations.go b/ee/tables/dism_default_associations/dism_default_associations.go index a73baa26b..0c2d3b253 100644 --- a/ee/tables/dism_default_associations/dism_default_associations.go +++ b/ee/tables/dism_default_associations/dism_default_associations.go @@ -82,12 +82,12 @@ func (t *Table) execDism(ctx context.Context) ([]byte, error) { if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "execing dism", + "args", args, "out", string(out), "err", err, - "args", args, ) - return nil, fmt.Errorf("execing dism: %w", err) + return nil, fmt.Errorf("execing dism: out: %s, %w", string(out), err) } data, err := os.ReadFile(filepath.Join(dir, dstFile)) diff --git a/ee/tables/filevault/filevault.go b/ee/tables/filevault/filevault.go index 63cff7694..bd08edc02 100644 --- a/ee/tables/filevault/filevault.go +++ b/ee/tables/filevault/filevault.go @@ -33,7 +33,7 @@ func TablePlugin(slogger *slog.Logger) *table.Plugin { } func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ([]map[string]string, error) { - output, err := tablehelpers.Exec(ctx, t.slogger, 10, allowedcmd.Fdesetup, []string{"status"}, false) + output, err := tablehelpers.RunSimple(ctx, t.slogger, 10, allowedcmd.Fdesetup, []string{"status"}) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "fdesetup failed", diff --git a/ee/tables/firmwarepasswd/firmwarepasswd.go b/ee/tables/firmwarepasswd/firmwarepasswd.go index b41d7b428..02eee3674 100644 --- a/ee/tables/firmwarepasswd/firmwarepasswd.go +++ b/ee/tables/firmwarepasswd/firmwarepasswd.go @@ -14,10 +14,10 @@ import ( "log/slog" "os" "strings" - "time" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/allowedcmd" + "github.com/kolide/launcher/ee/tables/tablehelpers" "github.com/osquery/osquery-go/plugin/table" ) @@ -70,8 +70,8 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( result := make(map[string]string) for _, mode := range []string{"-check", "-mode"} { - output := new(bytes.Buffer) - if err := t.runFirmwarepasswd(ctx, mode, output); err != nil { + out, err := t.runFirmwarepasswd(ctx, mode) + if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "error running firmware password", "command", mode, @@ -81,7 +81,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( } // Merge resulting matches - for _, row := range t.parser.Parse(output) { + for _, row := range t.parser.Parse(bytes.NewBuffer(out)) { for k, v := range row { result[k] = v } @@ -90,42 +90,29 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( return []map[string]string{result}, nil } -func (t *Table) runFirmwarepasswd(ctx context.Context, subcommand string, output *bytes.Buffer) error { - ctx, cancel := context.WithTimeout(ctx, 1*time.Second) - defer cancel() - - cmd, err := allowedcmd.Firmwarepasswd(ctx, subcommand) - if err != nil { - return fmt.Errorf("creating command: %w", err) - } - +func (t *Table) runFirmwarepasswd(ctx context.Context, subcommand string) ([]byte, error) { dir, err := agent.MkdirTemp("osq-firmwarepasswd") if err != nil { - return fmt.Errorf("mktemp: %w", err) + return nil, fmt.Errorf("mktemp: %w", err) } defer os.RemoveAll(dir) if err := os.Chmod(dir, 0755); err != nil { - return fmt.Errorf("chmod: %w", err) + return nil, fmt.Errorf("chmod: %w", err) } - cmd.Dir = dir - - stderr := new(bytes.Buffer) - cmd.Stderr = stderr - - cmd.Stdout = output - - if err := cmd.Run(); err != nil { + var out bytes.Buffer + if err := tablehelpers.Run(ctx, t.slogger, 1, allowedcmd.Firmwarepasswd, []string{subcommand}, &out, &out, tablehelpers.WithDir(dir)); err != nil { t.slogger.Log(ctx, slog.LevelDebug, - "error running firmwarepasswd", - "stderr", strings.TrimSpace(stderr.String()), - "stdout", strings.TrimSpace(output.String()), + "execing firmwarepasswd", + "subcommand", subcommand, + "out", out.String(), "err", err, ) - return fmt.Errorf("running firmwarepasswd: %w", err) + return out.Bytes(), fmt.Errorf("execing firmwarepasswd: %w", err) } - return nil + + return out.Bytes(), nil } func modeValue(in string) (string, error) { diff --git a/ee/tables/gsettings/gsettings.go b/ee/tables/gsettings/gsettings.go index 7c0840e76..e1e260504 100644 --- a/ee/tables/gsettings/gsettings.go +++ b/ee/tables/gsettings/gsettings.go @@ -13,10 +13,7 @@ import ( "log/slog" "os" "os/user" - "strconv" "strings" - "syscall" - "time" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/allowedcmd" @@ -26,7 +23,7 @@ import ( const allowedCharacters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-." -type gsettingsExecer func(ctx context.Context, username string, buf *bytes.Buffer) error +type gsettingsExecer func(ctx context.Context, slogger *slog.Logger, username string, buf *bytes.Buffer) error type GsettingsValues struct { slogger *slog.Logger @@ -61,7 +58,7 @@ func (t *GsettingsValues) generate(ctx context.Context, queryContext table.Query for _, username := range users { var output bytes.Buffer - err := t.getBytes(ctx, username, &output) + err := t.getBytes(ctx, t.slogger, username, &output) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "error getting bytes for user", @@ -80,46 +77,12 @@ func (t *GsettingsValues) generate(ctx context.Context, queryContext table.Query // execGsettings writes the output of running 'gsettings' command into the // supplied bytes buffer -func execGsettings(ctx context.Context, username string, buf *bytes.Buffer) error { - ctx, cancel := context.WithTimeout(ctx, 2*time.Second) - defer cancel() - +func execGsettings(ctx context.Context, slogger *slog.Logger, username string, buf *bytes.Buffer) error { u, err := user.Lookup(username) if err != nil { return fmt.Errorf("finding user by username '%s': %w", username, err) } - cmd, err := allowedcmd.Gsettings(ctx, "list-recursively") - if err != nil { - return fmt.Errorf("creating gsettings command: %w", err) - } - - // set the HOME for the the cmd so that gsettings is exec'd properly as the - // new user. - cmd.Env = append(cmd.Env, fmt.Sprintf("HOME=%s", u.HomeDir)) - - // Check if the supplied UID is that of the current user - currentUser, err := user.Current() - if err != nil { - return fmt.Errorf("checking current user uid: %w", err) - } - - if u.Uid != currentUser.Uid { - uid, err := strconv.ParseInt(u.Uid, 10, 32) - if err != nil { - return fmt.Errorf("converting uid from string to int: %w", err) - } - gid, err := strconv.ParseInt(u.Gid, 10, 32) - if err != nil { - return fmt.Errorf("converting gid from string to int: %w", err) - } - cmd.SysProcAttr = &syscall.SysProcAttr{} - cmd.SysProcAttr.Credential = &syscall.Credential{ - Uid: uint32(uid), - Gid: uint32(gid), - } - } - dir, err := agent.MkdirTemp("osq-gsettings") if err != nil { return fmt.Errorf("mktemp: %w", err) @@ -132,14 +95,15 @@ func execGsettings(ctx context.Context, username string, buf *bytes.Buffer) erro return fmt.Errorf("chmod: %w", err) } - cmd.Dir = dir + var stderr bytes.Buffer - stderr := new(bytes.Buffer) - cmd.Stderr = stderr - cmd.Stdout = buf - - if err := cmd.Run(); err != nil { - return fmt.Errorf("running gsettings, err is: %s: %w", stderr.String(), err) + if err := tablehelpers.Run(ctx, slogger, 2, + allowedcmd.Gsettings, []string{"list-recursively"}, buf, &stderr, + tablehelpers.WithUid(u.Uid), + tablehelpers.WithAppendEnv("HOME", u.HomeDir), + tablehelpers.WithDir(dir), + ); err != nil { + return fmt.Errorf("creating gsettings command: %w", err) } return nil diff --git a/ee/tables/gsettings/gsettings_test.go b/ee/tables/gsettings/gsettings_test.go index aa5068d59..627449b0f 100644 --- a/ee/tables/gsettings/gsettings_test.go +++ b/ee/tables/gsettings/gsettings_test.go @@ -6,6 +6,7 @@ package gsettings import ( "bytes" "context" + "log/slog" "os" "path/filepath" "testing" @@ -49,7 +50,7 @@ func TestGsettingsValues(t *testing.T) { tt := tt table := GsettingsValues{ slogger: multislogger.NewNopLogger(), - getBytes: func(ctx context.Context, username string, buf *bytes.Buffer) error { + getBytes: func(ctx context.Context, slogger *slog.Logger, username string, buf *bytes.Buffer) error { f, err := os.Open(filepath.Join("testdata", tt.filename)) require.NoError(t, err, "opening file %s", tt.filename) _, err = buf.ReadFrom(f) diff --git a/ee/tables/homebrew/upgradeable.go b/ee/tables/homebrew/upgradeable.go index 92f9ea183..cb1a5af3f 100644 --- a/ee/tables/homebrew/upgradeable.go +++ b/ee/tables/homebrew/upgradeable.go @@ -4,6 +4,7 @@ package brew_upgradeable import ( + "bytes" "context" "fmt" "log/slog" @@ -45,9 +46,15 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( for _, uid := range uids { for _, dataQuery := range tablehelpers.GetConstraints(queryContext, "query", tablehelpers.WithDefaults("*")) { // Brew can take a while to load the first time the command is ran, so leaving 60 seconds for the timeout here. - output, err := tablehelpers.Exec(ctx, t.slogger, 60, allowedcmd.Brew, []string{"outdated", "--json"}, true, tablehelpers.WithUid(uid)) - if err != nil { - t.slogger.Log(ctx, slog.LevelInfo, "failure querying user brew installed packages", "err", err, "target_uid", uid) + + var output bytes.Buffer + if err := tablehelpers.Run(ctx, t.slogger, 60, allowedcmd.Brew, []string{"update"}, &output, &output, tablehelpers.WithUid(uid)); err != nil { + t.slogger.Log(ctx, slog.LevelInfo, + "failure querying user brew installed packages", + "err", err, + "target_uid", uid, + "output", output.String(), + ) continue } @@ -56,7 +63,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( dataflatten.WithQuery(strings.Split(dataQuery, "/")), } - flattened, err := dataflatten.Json(output, flattenOpts...) + flattened, err := dataflatten.Json(output.Bytes(), flattenOpts...) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "failure flattening output", "err", err) continue diff --git a/ee/tables/ioreg/ioreg.go b/ee/tables/ioreg/ioreg.go index 3ea251810..2d1415edd 100644 --- a/ee/tables/ioreg/ioreg.go +++ b/ee/tables/ioreg/ioreg.go @@ -102,7 +102,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( for _, dataQuery := range tablehelpers.GetConstraints(queryContext, "query", tablehelpers.WithDefaults("*")) { // Finally, an inner loop - ioregOutput, err := tablehelpers.Exec(ctx, t.slogger, 30, allowedcmd.Ioreg, ioregArgs, false) + ioregOutput, err := tablehelpers.RunSimple(ctx, t.slogger, 30, allowedcmd.Ioreg, ioregArgs) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "ioreg failed", diff --git a/ee/tables/mdmclient/mdmclient.go b/ee/tables/mdmclient/mdmclient.go index 5464489c9..0577d2400 100644 --- a/ee/tables/mdmclient/mdmclient.go +++ b/ee/tables/mdmclient/mdmclient.go @@ -91,7 +91,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( for _, dataQuery := range tablehelpers.GetConstraints(queryContext, "query", tablehelpers.WithDefaults("*")) { - mdmclientOutput, err := tablehelpers.Exec(ctx, t.slogger, 30, allowedcmd.Mdmclient, []string{mdmclientCommand}, false) + mdmclientOutput, err := tablehelpers.RunSimple(ctx, t.slogger, 30, allowedcmd.Mdmclient, []string{mdmclientCommand}) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "mdmclient failed", diff --git a/ee/tables/profiles/profiles.go b/ee/tables/profiles/profiles.go index f7f9a5b57..dd1392882 100644 --- a/ee/tables/profiles/profiles.go +++ b/ee/tables/profiles/profiles.go @@ -101,7 +101,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( return nil, fmt.Errorf("Unknown user argument: %s", user) } - output, err := tablehelpers.Exec(ctx, t.slogger, 30, allowedcmd.Profiles, profileArgs, false) + output, err := tablehelpers.RunSimple(ctx, t.slogger, 30, allowedcmd.Profiles, profileArgs) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "ioreg exec failed", diff --git a/ee/tables/spotlight/spotlight.go b/ee/tables/spotlight/spotlight.go index da02eb7af..445418d9b 100644 --- a/ee/tables/spotlight/spotlight.go +++ b/ee/tables/spotlight/spotlight.go @@ -57,7 +57,7 @@ func (t *spotlightTable) generate(ctx context.Context, queryContext table.QueryC query = []string{where} } - out, err := tablehelpers.Exec(ctx, t.slogger, 120, allowedcmd.Mdfind, query, false) + out, err := tablehelpers.RunSimple(ctx, t.slogger, 120, allowedcmd.Mdfind, query) if err != nil { return nil, fmt.Errorf("call mdfind: %w", err) } diff --git a/ee/tables/tablehelpers/exec.go b/ee/tables/tablehelpers/exec.go index 50dcfba98..30987f385 100644 --- a/ee/tables/tablehelpers/exec.go +++ b/ee/tables/tablehelpers/exec.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "io" "log/slog" "os" "os/exec" @@ -26,36 +27,44 @@ func WithDir(dir string) ExecOps { } } -// Exec is a wrapper over exec.CommandContext. It does a couple of -// additional things to help with table usage: -// 1. It enforces a timeout. -// 2. Second, it accepts an array of possible binaries locations, and if something is not -// found, it will go down the list. -// 3. It moves the stderr into the return error, if needed. -// -// This is not suitable for high performance work -- it allocates new buffers each time. +func WithAppendEnv(key, value string) ExecOps { + return func(cmd *exec.Cmd) error { + cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", key, value)) + return nil + } +} + +// RunSimple is a wrapper over allowedcmd.AllowedCommand. +// It enforces a timeout, logs, traces, and returns the stdout as a byte slice. // -// `possibleBins` can be either a list of command names, or a list of paths to commands. -// Where reasonable, `possibleBins` should be command names only, so that we can perform -// lookup against PATH. -func Exec(ctx context.Context, slogger *slog.Logger, timeoutSeconds int, execCmd allowedcmd.AllowedCommand, args []string, includeStderr bool, opts ...ExecOps) ([]byte, error) { +// This is not suitable for high performance work -- it allocates new buffers each time +// Use Run() for more control over the stdout and stderr streams. +func RunSimple(ctx context.Context, slogger *slog.Logger, timeoutSeconds int, cmd allowedcmd.AllowedCommand, args []string, opts ...ExecOps) ([]byte, error) { + var stdout bytes.Buffer + if err := Run(ctx, slogger, timeoutSeconds, cmd, args, &stdout, io.Discard, opts...); err != nil { + return nil, err + } + + return stdout.Bytes(), nil +} + +// Run is a wrapper over allowedcmd.AllowedCommand. It enforces a timeout, logs, traces. +// Use RunSimple() for a simpler interface. +func Run(ctx context.Context, slogger *slog.Logger, timeoutSeconds int, execCmd allowedcmd.AllowedCommand, args []string, stdout io.Writer, stderr io.Writer, opts ...ExecOps) error { ctx, span := traces.StartSpan(ctx) defer span.End() ctx, cancel := context.WithTimeout(ctx, time.Duration(timeoutSeconds)*time.Second) defer cancel() - var stdout bytes.Buffer - var stderr bytes.Buffer - cmd, err := execCmd(ctx, args...) if err != nil { - return nil, fmt.Errorf("creating command: %w", err) + return fmt.Errorf("creating command: %w", err) } for _, opt := range opts { if err := opt(cmd); err != nil { - return nil, err + return fmt.Errorf("applying option: %w", err) } } @@ -63,29 +72,27 @@ func Exec(ctx context.Context, slogger *slog.Logger, timeoutSeconds int, execCmd span.SetAttributes(attribute.String("exec.binary", filepath.Base(cmd.Path))) span.SetAttributes(attribute.StringSlice("exec.args", args)) - cmd.Stdout = &stdout - if includeStderr { - cmd.Stderr = &stdout - } else { - cmd.Stderr = &stderr - } + cmd.Stdout = stdout + cmd.Stderr = stderr slogger.Log(ctx, slog.LevelDebug, "execing", "cmd", cmd.String(), + "args", args, + "timeout", timeoutSeconds, ) switch err := cmd.Run(); { case err == nil: - return stdout.Bytes(), nil + return nil case os.IsNotExist(err): - return nil, fmt.Errorf("could not find %s to run: %w", cmd.Path, err) + return fmt.Errorf("could not find %s to run: %w", cmd.Path, err) case ctx.Err() != nil: // ctx.Err() should only be set if the context is canceled or done traces.SetError(span, ctx.Err()) - return nil, fmt.Errorf("context canceled during exec '%s'. Got: '%s': %w", cmd.String(), stderr.String(), ctx.Err()) + return fmt.Errorf("context canceled during exec '%s': %w", cmd.String(), ctx.Err()) default: traces.SetError(span, err) - return nil, fmt.Errorf("exec '%s'. Got: '%s': %w", cmd.String(), stderr.String(), err) + return fmt.Errorf("exec '%s': %w", cmd.String(), err) } } diff --git a/ee/tables/tablehelpers/exec_test.go b/ee/tables/tablehelpers/exec_test.go index dd463dfae..d75a60b14 100644 --- a/ee/tables/tablehelpers/exec_test.go +++ b/ee/tables/tablehelpers/exec_test.go @@ -4,52 +4,118 @@ package tablehelpers import ( + "bytes" "context" "testing" "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/pkg/log/multislogger" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestExec(t *testing.T) { +func TestRun(t *testing.T) { t.Parallel() - var tests = []struct { - name string - timeout int - bin allowedcmd.AllowedCommand - args []string - err bool - output string + tests := []struct { + name string + timeoutSeconds int + execCmd allowedcmd.AllowedCommand + args []string + wantStdout bool + wantStderr bool + opts []ExecOps + assertion assert.ErrorAssertionFunc }{ { - name: "output", - bin: allowedcmd.Echo, - args: []string{"hello"}, - output: "hello\n", + name: "happy path", + timeoutSeconds: 1, + execCmd: allowedcmd.Echo, + args: []string{"hi"}, + wantStdout: true, + wantStderr: false, + assertion: assert.NoError, + }, + { + name: "timeout", + timeoutSeconds: 0, + execCmd: allowedcmd.Echo, + args: []string{"hi"}, + wantStdout: false, + wantStderr: false, + assertion: assert.Error, + }, + { + name: "stderr", + timeoutSeconds: 1, + execCmd: allowedcmd.Ps, + args: []string{"-z"}, + wantStdout: false, + wantStderr: true, + assertion: assert.Error, }, } - - ctx := context.Background() - slogger := multislogger.NewNopLogger() - for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - if tt.timeout == 0 { - tt.timeout = 30 + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + tt.assertion(t, Run(context.TODO(), multislogger.NewNopLogger(), tt.timeoutSeconds, tt.execCmd, tt.args, stdout, stderr)) + + if tt.wantStdout { + require.NotEmpty(t, stdout.String()) + } else { + require.Empty(t, stdout.String()) } - output, err := Exec(ctx, slogger, tt.timeout, tt.bin, tt.args, false) - if tt.err { - assert.Error(t, err) - assert.Empty(t, output) + + if tt.wantStderr { + require.NotEmpty(t, stderr.String()) } else { - assert.NoError(t, err) - assert.Equal(t, []byte(tt.output), output) + require.Empty(t, stderr.String()) } }) } } + +func TestRunSimple(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + timeoutSeconds int + cmd allowedcmd.AllowedCommand + args []string + opts []ExecOps + want []byte + assertion assert.ErrorAssertionFunc + }{ + { + name: "happy path", + timeoutSeconds: 1, + cmd: allowedcmd.Echo, + args: []string{"hi"}, + want: []byte("hi\n"), + assertion: assert.NoError, + }, + { + name: "error", + timeoutSeconds: 1, + cmd: allowedcmd.Ps, + args: []string{"-z"}, + want: nil, + assertion: assert.Error, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := RunSimple(context.TODO(), multislogger.NewNopLogger(), tt.timeoutSeconds, tt.cmd, tt.args, tt.opts...) + tt.assertion(t, err) + require.Equal(t, tt.want, got) + }) + } +} diff --git a/ee/tables/zfs/tables.go b/ee/tables/zfs/tables.go index e323e80d8..66b587bb6 100644 --- a/ee/tables/zfs/tables.go +++ b/ee/tables/zfs/tables.go @@ -73,7 +73,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( args = append(args, names...) - output, err := tablehelpers.Exec(ctx, t.slogger, 15, t.cmd, args, false) + output, err := tablehelpers.RunSimple(ctx, t.slogger, 15, t.cmd, args) if err != nil { // exec will error if there's no binary, so we never want to record that if os.IsNotExist(errors.Cause(err)) { diff --git a/pkg/osquery/table/touchid_system_darwin.go b/pkg/osquery/table/touchid_system_darwin.go index 9a3549e8f..9f1563aa2 100644 --- a/pkg/osquery/table/touchid_system_darwin.go +++ b/pkg/osquery/table/touchid_system_darwin.go @@ -34,7 +34,7 @@ func (t *touchIDSystemConfigTable) generate(ctx context.Context, queryContext ta var results []map[string]string var touchIDCompatible, secureEnclaveCPU, touchIDEnabled, touchIDUnlock string - stdout, err := tablehelpers.Exec(ctx, t.slogger, 10, allowedcmd.SystemProfiler, []string{"SPiBridgeDataType"}, false) + stdout, err := tablehelpers.RunSimple(ctx, t.slogger, 10, allowedcmd.SystemProfiler, []string{"SPiBridgeDataType"}) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "execing system_profiler SPiBridgeDataType", @@ -53,7 +53,7 @@ func (t *touchIDSystemConfigTable) generate(ctx context.Context, queryContext ta } // Read the system's bioutil configuration - stdout, err = tablehelpers.Exec(ctx, t.slogger, 10, allowedcmd.Bioutil, []string{"-r", "-s"}, false) + stdout, err = tablehelpers.RunSimple(ctx, t.slogger, 10, allowedcmd.Bioutil, []string{"-r", "-s"}) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "execing bioutil", diff --git a/pkg/osquery/table/touchid_user_darwin.go b/pkg/osquery/table/touchid_user_darwin.go index 639963a4e..948b29099 100644 --- a/pkg/osquery/table/touchid_user_darwin.go +++ b/pkg/osquery/table/touchid_user_darwin.go @@ -57,7 +57,7 @@ func (t *touchIDUserConfigTable) generate(ctx context.Context, queryContext tabl } // Get the user's TouchID config - configOutput, err := tablehelpers.Exec(ctx, t.slogger, 10, allowedcmd.Bioutil, []string{"-r"}, false, tablehelpers.WithUid(u.Uid)) + configOutput, err := tablehelpers.RunSimple(ctx, t.slogger, 10, allowedcmd.Bioutil, []string{"-r"}, tablehelpers.WithUid(u.Uid)) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "could not run bioutil -r", @@ -87,7 +87,7 @@ func (t *touchIDUserConfigTable) generate(ctx context.Context, queryContext tabl } // Grab the fingerprint count - countOut, err := tablehelpers.Exec(ctx, t.slogger, 10, allowedcmd.Bioutil, []string{"-c"}, false, tablehelpers.WithUid(u.Uid)) + countOut, err := tablehelpers.RunSimple(ctx, t.slogger, 10, allowedcmd.Bioutil, []string{"-c"}, tablehelpers.WithUid(u.Uid)) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "could not run bioutil -c", From 094e226140855502b8bf9df8ea8b792d0b3308f4 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 2 May 2024 15:28:42 -0700 Subject: [PATCH 06/14] update linux execs --- ee/tables/crowdstrike/falcon_kernel_check/table.go | 2 +- ee/tables/cryptsetup/table.go | 2 +- ee/tables/nix_env/upgradeable/upgradeable.go | 8 +++++--- ee/tables/tablehelpers/exec.go | 1 + 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ee/tables/crowdstrike/falcon_kernel_check/table.go b/ee/tables/crowdstrike/falcon_kernel_check/table.go index a9dc39128..16097625d 100644 --- a/ee/tables/crowdstrike/falcon_kernel_check/table.go +++ b/ee/tables/crowdstrike/falcon_kernel_check/table.go @@ -35,7 +35,7 @@ func TablePlugin(slogger *slog.Logger) *table.Plugin { } func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ([]map[string]string, error) { - output, err := tablehelpers.Exec(ctx, t.slogger, 5, allowedcmd.FalconKernelCheck, []string{}, false) + output, err := tablehelpers.RunSimple(ctx, t.slogger, 5, allowedcmd.FalconKernelCheck, []string{}) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "exec failed", diff --git a/ee/tables/cryptsetup/table.go b/ee/tables/cryptsetup/table.go index 8ffbe32e4..e730df1d1 100644 --- a/ee/tables/cryptsetup/table.go +++ b/ee/tables/cryptsetup/table.go @@ -49,7 +49,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( } for _, name := range requestedNames { - output, err := tablehelpers.Exec(ctx, t.slogger, 15, allowedcmd.Cryptsetup, []string{"--readonly", "status", name}, false) + output, err := tablehelpers.RunSimple(ctx, t.slogger, 15, allowedcmd.Cryptsetup, []string{"--readonly", "status", name}) if err != nil { t.slogger.Log(ctx, slog.LevelDebug, "error execing for status", diff --git a/ee/tables/nix_env/upgradeable/upgradeable.go b/ee/tables/nix_env/upgradeable/upgradeable.go index 61cdff126..be135d1da 100644 --- a/ee/tables/nix_env/upgradeable/upgradeable.go +++ b/ee/tables/nix_env/upgradeable/upgradeable.go @@ -4,6 +4,7 @@ package nix_env_upgradeable import ( + "bytes" "context" "fmt" "log/slog" @@ -45,8 +46,9 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( for _, uid := range uids { for _, dataQuery := range tablehelpers.GetConstraints(queryContext, "query", tablehelpers.WithDefaults("*")) { // Nix takes a while to load, so leaving a minute timeout here to give enough time. More might be needed. - output, err := tablehelpers.Exec(ctx, t.slogger, 60, allowedcmd.NixEnv, []string{"--query", "--installed", "-c", "--xml"}, true, tablehelpers.WithUid(uid)) - if err != nil { + + var output bytes.Buffer + if err := tablehelpers.Run(ctx, t.slogger, 60, allowedcmd.NixEnv, []string{"--query", "--upgradeable", "-c", "--xml"}, &output, &output, tablehelpers.WithUid(uid)); err != nil { t.slogger.Log(ctx, slog.LevelInfo, "failure querying user installed packages", "err", err, "target_uid", uid) continue } @@ -56,7 +58,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( dataflatten.WithQuery(strings.Split(dataQuery, "/")), } - flattened, err := dataflatten.Xml(output, flattenOpts...) + flattened, err := dataflatten.Xml(output.Bytes(), flattenOpts...) if err != nil { t.slogger.Log(ctx, slog.LevelInfo, "failure flattening output", "err", err) continue diff --git a/ee/tables/tablehelpers/exec.go b/ee/tables/tablehelpers/exec.go index 30987f385..3316bce97 100644 --- a/ee/tables/tablehelpers/exec.go +++ b/ee/tables/tablehelpers/exec.go @@ -36,6 +36,7 @@ func WithAppendEnv(key, value string) ExecOps { // RunSimple is a wrapper over allowedcmd.AllowedCommand. // It enforces a timeout, logs, traces, and returns the stdout as a byte slice. +// It discards stderr. // // This is not suitable for high performance work -- it allocates new buffers each time // Use Run() for more control over the stdout and stderr streams. From 21bb9bf3f861670684a1951c836fe1afb0ec28d0 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 2 May 2024 15:37:29 -0700 Subject: [PATCH 07/14] fix windows --- .../dism_default_associations.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ee/tables/dism_default_associations/dism_default_associations.go b/ee/tables/dism_default_associations/dism_default_associations.go index 0c2d3b253..1fb3ed654 100644 --- a/ee/tables/dism_default_associations/dism_default_associations.go +++ b/ee/tables/dism_default_associations/dism_default_associations.go @@ -4,6 +4,7 @@ package dism_default_associations import ( + "bytes" "context" "fmt" "log/slog" @@ -78,16 +79,16 @@ func (t *Table) execDism(ctx context.Context) ([]byte, error) { const dstFile = "associations.xml" args := []string{"/online", "/Export-DefaultAppAssociations:" + dstFile} - out, err := tablehelpers.Exec(ctx, t.slogger, 30, allowedcmd.Dism, args, true, tablehelpers.WithDir(dir)) - if err != nil { + var out bytes.Buffer + if err := tablehelpers.Run(ctx, t.slogger, 30, allowedcmd.Dism, args, &out, &out, tablehelpers.WithDir(dir)); err != nil { t.slogger.Log(ctx, slog.LevelDebug, "execing dism", "args", args, - "out", string(out), + "out", out.String(), "err", err, ) - return nil, fmt.Errorf("execing dism: out: %s, %w", string(out), err) + return nil, fmt.Errorf("execing dism: out: %s, %w", out.String(), err) } data, err := os.ReadFile(filepath.Join(dir, dstFile)) From 318f5cd5ec16d2cb37382484e915e2e76bd9e1cb Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 2 May 2024 15:40:04 -0700 Subject: [PATCH 08/14] fix more windows --- ee/tables/secedit/secedit.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/tables/secedit/secedit.go b/ee/tables/secedit/secedit.go index c3ea6a54d..2ed976823 100644 --- a/ee/tables/secedit/secedit.go +++ b/ee/tables/secedit/secedit.go @@ -4,6 +4,7 @@ package secedit import ( + "bytes" "context" "fmt" "io" @@ -108,8 +109,9 @@ func (t *Table) execSecedit(ctx context.Context, mergedPolicy bool) ([]byte, err args = append(args, "/mergedpolicy") } - if out, err := tablehelpers.Exec(ctx, t.slogger, 30, allowedcmd.Secedit, args, true); err != nil { - return nil, fmt.Errorf("calling secedit. Got: %s: %w", string(out), err) + var out bytes.Buffer + if err := tablehelpers.Run(ctx, t.slogger, 30, allowedcmd.Secedit, args, &out, &out); err != nil { + return nil, fmt.Errorf("calling secedit. Got: %s: %w", out.String(), err) } file, err := os.Open(dst) From bf8cd15fbbe58749bb9c9d8e968b236b489b6d24 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Fri, 3 May 2024 15:27:58 -0700 Subject: [PATCH 09/14] more table helper conversions --- ee/tables/gsettings/metadata.go | 15 ++----- ee/tables/pwpolicy/pwpolicy.go | 18 +------- ee/tables/systemprofiler/systemprofiler.go | 24 +++------- .../tablehelpers/exec_osquery_launchctl.go | 14 ++---- ee/tables/wifi_networks/wifi_networks.go | 15 +------ ee/tables/xrdb/xrdb.go | 45 ++++--------------- 6 files changed, 25 insertions(+), 106 deletions(-) diff --git a/ee/tables/gsettings/metadata.go b/ee/tables/gsettings/metadata.go index d38d5e1fa..b2cd04aea 100644 --- a/ee/tables/gsettings/metadata.go +++ b/ee/tables/gsettings/metadata.go @@ -9,6 +9,7 @@ import ( "context" "errors" "fmt" + "io" "log/slog" "os" "strings" @@ -17,6 +18,7 @@ import ( "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/tables/tablehelpers" + "github.com/kolide/launcher/pkg/log/multislogger" "github.com/osquery/osquery-go/plugin/table" ) @@ -223,17 +225,8 @@ func execGsettingsCommand(ctx context.Context, args []string, tmpdir string, out defer cancel() command := args[0] - cmd, err := allowedcmd.Gsettings(ctx, args...) - if err != nil { - return fmt.Errorf("creating gsettings command: %w", err) - } - - cmd.Dir = tmpdir - cmd.Stderr = new(bytes.Buffer) - cmd.Stdout = output - - if err := cmd.Run(); err != nil { - return fmt.Errorf("running gsettings %s: %w", command, err) + if err := tablehelpers.Run(ctx, multislogger.NewNopLogger(), 3, allowedcmd.Gsettings, args, output, io.Discard, tablehelpers.WithDir(tmpdir)); err != nil { + return fmt.Errorf("execing gsettings: %s: %w", command, err) } return nil diff --git a/ee/tables/pwpolicy/pwpolicy.go b/ee/tables/pwpolicy/pwpolicy.go index f5d3a8963..ce7dbaa3e 100644 --- a/ee/tables/pwpolicy/pwpolicy.go +++ b/ee/tables/pwpolicy/pwpolicy.go @@ -15,7 +15,6 @@ import ( "fmt" "log/slog" "strings" - "time" "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/dataflatten" @@ -96,22 +95,7 @@ func (t *Table) execPwpolicy(ctx context.Context, args []string) ([]byte, error) var stdout bytes.Buffer var stderr bytes.Buffer - ctx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - - cmd, err := t.execCC(ctx, args...) - if err != nil { - return nil, fmt.Errorf("creating command: %w", err) - } - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - t.slogger.Log(ctx, slog.LevelDebug, - "calling pwpolicy", - "args", cmd.Args, - ) - - if err := cmd.Run(); err != nil { + if err := tablehelpers.Run(ctx, t.slogger, 30, t.execCC, args, &stdout, &stderr); err != nil { return nil, fmt.Errorf("calling pwpolicy. Got: %s: %w", stderr.String(), err) } diff --git a/ee/tables/systemprofiler/systemprofiler.go b/ee/tables/systemprofiler/systemprofiler.go index c2a76d16d..4e55897a7 100644 --- a/ee/tables/systemprofiler/systemprofiler.go +++ b/ee/tables/systemprofiler/systemprofiler.go @@ -42,12 +42,12 @@ import ( "fmt" "log/slog" "strings" - "time" "github.com/groob/plist" "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/dataflatten" "github.com/kolide/launcher/ee/tables/dataflattentable" + "github.com/kolide/launcher/ee/tables/tablehelpers" "github.com/osquery/osquery-go/plugin/table" ) @@ -194,12 +194,10 @@ func (t *Table) getRowsFromOutput(ctx context.Context, dataQuery, detailLevel st } func (t *Table) execSystemProfiler(ctx context.Context, detailLevel string, subcommands []string) ([]byte, error) { - timeout := 45 * time.Second + timeoutSeconds := 45 if detailLevel == "full" { - timeout = 5 * time.Minute + timeoutSeconds = 60 * 5 } - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() var stdout bytes.Buffer var stderr bytes.Buffer @@ -212,20 +210,8 @@ func (t *Table) execSystemProfiler(ctx context.Context, detailLevel string, subc args = append(args, subcommands...) - cmd, err := allowedcmd.SystemProfiler(ctx, args...) - if err != nil { - return nil, fmt.Errorf("creating system_profiler command: %w", err) - } - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - t.slogger.Log(ctx, slog.LevelDebug, - "calling system_profiler", - "args", cmd.Args, - ) - - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("calling system_profiler. Got: %s: %w", stderr.String(), err) + if err := tablehelpers.Run(ctx, t.slogger, timeoutSeconds, allowedcmd.SystemProfiler, args, &stdout, &stderr); err != nil { + return nil, fmt.Errorf("execing system_profiler. Got: %s: %w", stderr.String(), err) } return stdout.Bytes(), nil diff --git a/ee/tables/tablehelpers/exec_osquery_launchctl.go b/ee/tables/tablehelpers/exec_osquery_launchctl.go index e38cff32d..84b38ea7c 100644 --- a/ee/tables/tablehelpers/exec_osquery_launchctl.go +++ b/ee/tables/tablehelpers/exec_osquery_launchctl.go @@ -15,6 +15,7 @@ import ( "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/allowedcmd" + "github.com/kolide/launcher/pkg/log/multislogger" ) // ExecOsqueryLaunchctl runs osquery under launchctl, in a user context. @@ -27,7 +28,7 @@ func ExecOsqueryLaunchctl(ctx context.Context, timeoutSeconds int, username stri return nil, fmt.Errorf("looking up username %s: %w", username, err) } - cmd, err := allowedcmd.Launchctl(ctx, + args := []string{ "asuser", targetUser.Uid, osqueryPath, @@ -39,9 +40,6 @@ func ExecOsqueryLaunchctl(ctx context.Context, timeoutSeconds int, username stri "-S", "--json", query, - ) - if err != nil { - return nil, fmt.Errorf("creating launchctl command: %w", err) } dir, err := agent.MkdirTemp("osq-launchctl") @@ -54,12 +52,8 @@ func ExecOsqueryLaunchctl(ctx context.Context, timeoutSeconds int, username stri return nil, fmt.Errorf("chmod: %w", err) } - cmd.Dir = dir - - stdout, stderr := new(bytes.Buffer), new(bytes.Buffer) - cmd.Stdout, cmd.Stderr = stdout, stderr - - if err := cmd.Run(); err != nil { + var stdout, stderr bytes.Buffer + if err := Run(ctx, multislogger.NewNopLogger(), timeoutSeconds, allowedcmd.Launchctl, args, &stdout, &stderr, WithDir(dir)); err != nil { return nil, fmt.Errorf("running osquery. Got: '%s': %w", stderr.String(), err) } diff --git a/ee/tables/wifi_networks/wifi_networks.go b/ee/tables/wifi_networks/wifi_networks.go index 2a839855e..40ad99dfd 100644 --- a/ee/tables/wifi_networks/wifi_networks.go +++ b/ee/tables/wifi_networks/wifi_networks.go @@ -17,6 +17,7 @@ import ( "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/dataflatten" "github.com/kolide/launcher/ee/tables/dataflattentable" + "github.com/kolide/launcher/ee/tables/tablehelpers" "github.com/osquery/osquery-go/plugin/table" ) @@ -70,11 +71,6 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( func execPwsh(slogger *slog.Logger) execer { return func(ctx context.Context, buf *bytes.Buffer) error { - // MS requires interfaces to complete network scans in <4 seconds, but - // that appears not to be consistent - ctx, cancel := context.WithTimeout(ctx, 45*time.Second) - defer cancel() - // write the c# code to a file, so the powershell script can load it // from there. This works around a size limit on args passed to // powershell.exe @@ -90,16 +86,9 @@ func execPwsh(slogger *slog.Logger) execer { } args := append([]string{"-NoProfile", "-NonInteractive"}, string(pwshScript)) - cmd, err := allowedcmd.Powershell(ctx, args...) - if err != nil { - return fmt.Errorf("creating powershell command: %w", err) - } - cmd.Dir = dir var stderr bytes.Buffer - cmd.Stdout = buf - cmd.Stderr = &stderr - err = cmd.Run() + err = tablehelpers.Run(ctx, slogger, 45, allowedcmd.Powershell, args, buf, &stderr, tablehelpers.WithDir(dir)) errOutput := stderr.String() // sometimes the powershell script logs errors to stderr, but returns a // successful execution code. diff --git a/ee/tables/xrdb/xrdb.go b/ee/tables/xrdb/xrdb.go index cbd2600b0..0c8b85d15 100644 --- a/ee/tables/xrdb/xrdb.go +++ b/ee/tables/xrdb/xrdb.go @@ -13,14 +13,13 @@ import ( "log/slog" "os" "os/user" - "strconv" "strings" - "syscall" "time" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/tables/tablehelpers" + "github.com/kolide/launcher/pkg/log/multislogger" "github.com/osquery/osquery-go/plugin/table" ) @@ -94,35 +93,7 @@ func execXRDB(ctx context.Context, displayNum, username string, buf *bytes.Buffe return fmt.Errorf("finding user by username '%s': %w", username, err) } - cmd, err := allowedcmd.Xrdb(ctx, "-display", displayNum, "-global", "-query") - if err != nil { - return fmt.Errorf("creating xrdb command: %w", err) - } - - // set the HOME cmd so that xrdb is exec'd properly as the new user. - cmd.Env = append(cmd.Env, fmt.Sprintf("HOME=%s", u.HomeDir)) - - // Check if the supplied UID is that of the current user - currentUser, err := user.Current() - if err != nil { - return fmt.Errorf("checking current user uid: %w", err) - } - - if u.Uid != currentUser.Uid { - uid, err := strconv.ParseInt(u.Uid, 10, 32) - if err != nil { - return fmt.Errorf("converting uid from string to int: %w", err) - } - gid, err := strconv.ParseInt(u.Gid, 10, 32) - if err != nil { - return fmt.Errorf("converting gid from string to int: %w", err) - } - cmd.SysProcAttr = &syscall.SysProcAttr{} - cmd.SysProcAttr.Credential = &syscall.Credential{ - Uid: uint32(uid), - Gid: uint32(gid), - } - } + var stderr bytes.Buffer dir, err := agent.MkdirTemp("osq-xrdb") if err != nil { @@ -133,12 +104,14 @@ func execXRDB(ctx context.Context, displayNum, username string, buf *bytes.Buffe if err := os.Chmod(dir, 0755); err != nil { return fmt.Errorf("chmod: %w", err) } - cmd.Dir = dir - stderr := new(bytes.Buffer) - cmd.Stderr = stderr - cmd.Stdout = buf - if err := cmd.Run(); err != nil { + args := []string{"-display", displayNum, "-global", "-query"} + // set the HOME cmd so that xrdb is exec'd properly as the new user. + if err := tablehelpers.Run(ctx, multislogger.NewNopLogger(), 45, allowedcmd.Xrdb, args, buf, &stderr, + tablehelpers.WithUid(u.Uid), + tablehelpers.WithAppendEnv("HOME", u.HomeDir), + tablehelpers.WithDir(dir), + ); err != nil { return fmt.Errorf("running xrdb, err is: %s: %w", stderr.String(), err) } From 38def5aa75e95344f3381f2cdd68fc5d387720db Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 6 May 2024 07:35:13 -0700 Subject: [PATCH 10/14] drop unneeded contexts, re re factor --- ee/tables/firmwarepasswd/firmwarepasswd.go | 28 +++++++++++----------- ee/tables/gsettings/metadata.go | 4 ---- ee/tables/xrdb/xrdb.go | 4 ---- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/ee/tables/firmwarepasswd/firmwarepasswd.go b/ee/tables/firmwarepasswd/firmwarepasswd.go index 02eee3674..b4c1ae517 100644 --- a/ee/tables/firmwarepasswd/firmwarepasswd.go +++ b/ee/tables/firmwarepasswd/firmwarepasswd.go @@ -70,8 +70,8 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( result := make(map[string]string) for _, mode := range []string{"-check", "-mode"} { - out, err := t.runFirmwarepasswd(ctx, mode) - if err != nil { + output := new(bytes.Buffer) + if err := t.runFirmwarepasswd(ctx, mode, output); err != nil { t.slogger.Log(ctx, slog.LevelInfo, "error running firmware password", "command", mode, @@ -81,7 +81,7 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( } // Merge resulting matches - for _, row := range t.parser.Parse(bytes.NewBuffer(out)) { + for _, row := range t.parser.Parse(output) { for k, v := range row { result[k] = v } @@ -90,29 +90,29 @@ func (t *Table) generate(ctx context.Context, queryContext table.QueryContext) ( return []map[string]string{result}, nil } -func (t *Table) runFirmwarepasswd(ctx context.Context, subcommand string) ([]byte, error) { +func (t *Table) runFirmwarepasswd(ctx context.Context, subcommand string, output *bytes.Buffer) error { dir, err := agent.MkdirTemp("osq-firmwarepasswd") if err != nil { - return nil, fmt.Errorf("mktemp: %w", err) + return fmt.Errorf("mktemp: %w", err) } defer os.RemoveAll(dir) if err := os.Chmod(dir, 0755); err != nil { - return nil, fmt.Errorf("chmod: %w", err) + return fmt.Errorf("chmod: %w", err) } - var out bytes.Buffer - if err := tablehelpers.Run(ctx, t.slogger, 1, allowedcmd.Firmwarepasswd, []string{subcommand}, &out, &out, tablehelpers.WithDir(dir)); err != nil { + stderr := new(bytes.Buffer) + + if err := tablehelpers.Run(ctx, t.slogger, 1, allowedcmd.Firmwarepasswd, []string{subcommand}, output, stderr, tablehelpers.WithDir(dir)); err != nil { t.slogger.Log(ctx, slog.LevelDebug, - "execing firmwarepasswd", - "subcommand", subcommand, - "out", out.String(), + "error running firmwarepasswd", + "stderr", strings.TrimSpace(stderr.String()), + "stdout", strings.TrimSpace(output.String()), "err", err, ) - return out.Bytes(), fmt.Errorf("execing firmwarepasswd: %w", err) + return fmt.Errorf("running firmwarepasswd: %w", err) } - - return out.Bytes(), nil + return nil } func modeValue(in string) (string, error) { diff --git a/ee/tables/gsettings/metadata.go b/ee/tables/gsettings/metadata.go index b2cd04aea..f47f224c2 100644 --- a/ee/tables/gsettings/metadata.go +++ b/ee/tables/gsettings/metadata.go @@ -13,7 +13,6 @@ import ( "log/slog" "os" "strings" - "time" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/allowedcmd" @@ -221,9 +220,6 @@ func (t *GsettingsMetadata) getType(ctx context.Context, schema, key, tmpdir str // execGsettingsCommand should be called with a tmpdir that will be cleaned up. func execGsettingsCommand(ctx context.Context, args []string, tmpdir string, output *bytes.Buffer) error { - ctx, cancel := context.WithTimeout(ctx, 3*time.Second) - defer cancel() - command := args[0] if err := tablehelpers.Run(ctx, multislogger.NewNopLogger(), 3, allowedcmd.Gsettings, args, output, io.Discard, tablehelpers.WithDir(tmpdir)); err != nil { return fmt.Errorf("execing gsettings: %s: %w", command, err) diff --git a/ee/tables/xrdb/xrdb.go b/ee/tables/xrdb/xrdb.go index 0c8b85d15..74199c5e9 100644 --- a/ee/tables/xrdb/xrdb.go +++ b/ee/tables/xrdb/xrdb.go @@ -14,7 +14,6 @@ import ( "os" "os/user" "strings" - "time" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/allowedcmd" @@ -85,9 +84,6 @@ func (t *XRDBSettings) generate(ctx context.Context, queryContext table.QueryCon // execXRDB writes the output of running 'xrdb' command into the // supplied bytes buffer func execXRDB(ctx context.Context, displayNum, username string, buf *bytes.Buffer) error { - ctx, cancel := context.WithTimeout(ctx, 2*time.Second) - defer cancel() - u, err := user.Lookup(username) if err != nil { return fmt.Errorf("finding user by username '%s': %w", username, err) From ab57abeeeb320078fd944a7fd4af3e0025b9b8f3 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 6 May 2024 07:41:16 -0700 Subject: [PATCH 11/14] simplify dism update --- .../dsim_default_associations.go} | 25 ++++++++++--------- pkg/osquery/table/platform_tables_windows.go | 4 +-- 2 files changed, 15 insertions(+), 14 deletions(-) rename ee/tables/{dism_default_associations/dism_default_associations.go => dsim_default_associations/dsim_default_associations.go} (83%) diff --git a/ee/tables/dism_default_associations/dism_default_associations.go b/ee/tables/dsim_default_associations/dsim_default_associations.go similarity index 83% rename from ee/tables/dism_default_associations/dism_default_associations.go rename to ee/tables/dsim_default_associations/dsim_default_associations.go index 1fb3ed654..d9bd9cff9 100644 --- a/ee/tables/dism_default_associations/dism_default_associations.go +++ b/ee/tables/dsim_default_associations/dsim_default_associations.go @@ -1,7 +1,7 @@ //go:build windows // +build windows -package dism_default_associations +package dsim_default_associations import ( "bytes" @@ -76,24 +76,25 @@ func (t *Table) execDism(ctx context.Context) ([]byte, error) { return nil, fmt.Errorf("creating kolide_dism tmp dir: %w", err) } defer os.RemoveAll(dir) - const dstFile = "associations.xml" + + dstFile := "associations.xml" + var stdout bytes.Buffer + var stderr bytes.Buffer + args := []string{"/online", "/Export-DefaultAppAssociations:" + dstFile} - var out bytes.Buffer - if err := tablehelpers.Run(ctx, t.slogger, 30, allowedcmd.Dism, args, &out, &out, tablehelpers.WithDir(dir)); err != nil { - t.slogger.Log(ctx, slog.LevelDebug, - "execing dism", - "args", args, - "out", out.String(), - "err", err, - ) + t.slogger.Log(ctx, slog.LevelDebug, + "calling dsim", + "args", args, + ) - return nil, fmt.Errorf("execing dism: out: %s, %w", out.String(), err) + if err := tablehelpers.Run(ctx, t.slogger, 30, allowedcmd.Dism, args, &stdout, &stderr, tablehelpers.WithDir(dir)); err != nil { + return nil, fmt.Errorf("calling dism. Got: %s: %w", stderr.String(), err) } data, err := os.ReadFile(filepath.Join(dir, dstFile)) if err != nil { - return nil, fmt.Errorf("reading dism output file: %w", err) + return nil, fmt.Errorf("error reading dism output file: %s: %w", err, err) } return data, nil diff --git a/pkg/osquery/table/platform_tables_windows.go b/pkg/osquery/table/platform_tables_windows.go index 2ee6f8287..f41e336e4 100644 --- a/pkg/osquery/table/platform_tables_windows.go +++ b/pkg/osquery/table/platform_tables_windows.go @@ -8,7 +8,7 @@ import ( "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/tables/dataflattentable" - "github.com/kolide/launcher/ee/tables/dism_default_associations" + "github.com/kolide/launcher/ee/tables/dsim_default_associations" "github.com/kolide/launcher/ee/tables/execparsers/dsregcmd" "github.com/kolide/launcher/ee/tables/secedit" "github.com/kolide/launcher/ee/tables/wifi_networks" @@ -20,7 +20,7 @@ import ( func platformSpecificTables(slogger *slog.Logger, currentOsquerydBinaryPath string) []osquery.OsqueryPlugin { return []osquery.OsqueryPlugin{ ProgramIcons(), - dism_default_associations.TablePlugin(slogger), + dsim_default_associations.TablePlugin(slogger), secedit.TablePlugin(slogger), wifi_networks.TablePlugin(slogger), windowsupdatetable.TablePlugin(windowsupdatetable.UpdatesTable, slogger), From 92a9a168c91a01501fabc83c6d4b66e126d358f4 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 6 May 2024 08:03:13 -0700 Subject: [PATCH 12/14] make changes smaller --- ee/tables/pwpolicy/pwpolicy.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ee/tables/pwpolicy/pwpolicy.go b/ee/tables/pwpolicy/pwpolicy.go index ce7dbaa3e..3b3323ed3 100644 --- a/ee/tables/pwpolicy/pwpolicy.go +++ b/ee/tables/pwpolicy/pwpolicy.go @@ -95,6 +95,11 @@ func (t *Table) execPwpolicy(ctx context.Context, args []string) ([]byte, error) var stdout bytes.Buffer var stderr bytes.Buffer + t.slogger.Log(ctx, slog.LevelDebug, + "calling pwpolicy", + "args", args, + ) + if err := tablehelpers.Run(ctx, t.slogger, 30, t.execCC, args, &stdout, &stderr); err != nil { return nil, fmt.Errorf("calling pwpolicy. Got: %s: %w", stderr.String(), err) } From 74c6732a70698a41ca46e194f360474322f19a2a Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 6 May 2024 08:08:48 -0700 Subject: [PATCH 13/14] make changes smaller --- ee/tables/systemprofiler/systemprofiler.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ee/tables/systemprofiler/systemprofiler.go b/ee/tables/systemprofiler/systemprofiler.go index 4e55897a7..0ddea3a7f 100644 --- a/ee/tables/systemprofiler/systemprofiler.go +++ b/ee/tables/systemprofiler/systemprofiler.go @@ -196,7 +196,7 @@ func (t *Table) getRowsFromOutput(ctx context.Context, dataQuery, detailLevel st func (t *Table) execSystemProfiler(ctx context.Context, detailLevel string, subcommands []string) ([]byte, error) { timeoutSeconds := 45 if detailLevel == "full" { - timeoutSeconds = 60 * 5 + timeoutSeconds = 5 * 60 } var stdout bytes.Buffer @@ -210,8 +210,13 @@ func (t *Table) execSystemProfiler(ctx context.Context, detailLevel string, subc args = append(args, subcommands...) + t.slogger.Log(ctx, slog.LevelDebug, + "calling system_profiler", + "args", args, + ) + if err := tablehelpers.Run(ctx, t.slogger, timeoutSeconds, allowedcmd.SystemProfiler, args, &stdout, &stderr); err != nil { - return nil, fmt.Errorf("execing system_profiler. Got: %s: %w", stderr.String(), err) + return nil, fmt.Errorf("calling system_profiler. Got: %s: %w", stderr.String(), err) } return stdout.Bytes(), nil From d73ed42d35b8e0d54adea077467c75d62ecf0a81 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 6 May 2024 08:13:00 -0700 Subject: [PATCH 14/14] remove unneeded ctx --- ee/tables/tablehelpers/exec_osquery_launchctl.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ee/tables/tablehelpers/exec_osquery_launchctl.go b/ee/tables/tablehelpers/exec_osquery_launchctl.go index 84b38ea7c..794f44af1 100644 --- a/ee/tables/tablehelpers/exec_osquery_launchctl.go +++ b/ee/tables/tablehelpers/exec_osquery_launchctl.go @@ -11,7 +11,6 @@ import ( "log/slog" "os" "os/user" - "time" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/allowedcmd" @@ -20,9 +19,6 @@ import ( // ExecOsqueryLaunchctl runs osquery under launchctl, in a user context. func ExecOsqueryLaunchctl(ctx context.Context, timeoutSeconds int, username string, osqueryPath string, query string) ([]byte, error) { - ctx, cancel := context.WithTimeout(ctx, time.Duration(timeoutSeconds)*time.Second) - defer cancel() - targetUser, err := user.Lookup(username) if err != nil { return nil, fmt.Errorf("looking up username %s: %w", username, err) @@ -58,7 +54,6 @@ func ExecOsqueryLaunchctl(ctx context.Context, timeoutSeconds int, username stri } return stdout.Bytes(), nil - } func ExecOsqueryLaunchctlParsed(ctx context.Context, slogger *slog.Logger, timeoutSeconds int, username string, osqueryPath string, query string) ([]map[string]string, error) {