From 9d056052cc1fc698b9b5e49b1a786ddd24fa4b2e Mon Sep 17 00:00:00 2001 From: James Pickett Date: Fri, 13 Sep 2024 13:50:02 -0700 Subject: [PATCH 01/23] first pass poc at macos presence detection --- ee/presence/presence_darwin.go | 102 ++++++++++++++++++++++++++++ ee/presence/presence_darwin_test.go | 48 +++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 ee/presence/presence_darwin.go create mode 100644 ee/presence/presence_darwin_test.go diff --git a/ee/presence/presence_darwin.go b/ee/presence/presence_darwin.go new file mode 100644 index 000000000..1264d4c52 --- /dev/null +++ b/ee/presence/presence_darwin.go @@ -0,0 +1,102 @@ +package presence + +/* +#cgo CFLAGS: -x objective-c -fmodules -fblocks +#cgo LDFLAGS: -framework CoreFoundation -framework LocalAuthentication -framework Foundation +#include +#include +#import + +struct AuthResult { + bool success; // true for success, false for failure + char* error_msg; // Error message if any + int error_code; // Error code if any +}; + +struct AuthResult Authenticate(char const* reason) { + struct AuthResult authResult; + LAContext *myContext = [[LAContext alloc] init]; + NSError *authError = nil; + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + NSString *nsReason = [NSString stringWithUTF8String:reason]; + __block bool success = false; + __block NSString *errorMessage = nil; + __block int errorCode = 0; + + // Use LAPolicyDeviceOwnerAuthentication to allow biometrics and password fallback + if ([myContext canEvaluatePolicy:LAPolicyDeviceOwnerAuthentication error:&authError]) { + [myContext evaluatePolicy:LAPolicyDeviceOwnerAuthentication + localizedReason:nsReason + reply:^(BOOL policySuccess, NSError *error) { + if (policySuccess) { + success = true; // Authentication successful + } else { + success = false; + errorCode = (int)[error code]; + errorMessage = [error localizedDescription]; + if (error.code == LAErrorUserFallback || error.code == LAErrorAuthenticationFailed) { + // Prompting for password + [myContext evaluatePolicy:LAPolicyDeviceOwnerAuthentication + localizedReason:@"Please enter your password" + reply:^(BOOL pwdSuccess, NSError *error) { + if (pwdSuccess) { + success = true; + } else { + success = false; + errorCode = (int)[error code]; + errorMessage = [error localizedDescription]; + } + dispatch_semaphore_signal(sema); + }]; + } else { + errorCode = (int)[error code]; + errorMessage = [error localizedDescription]; + } + } + dispatch_semaphore_signal(sema); + }]; + } else { + success = false; // Cannot evaluate policy + errorCode = (int)[authError code]; + errorMessage = [authError localizedDescription]; + } + + dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER); + dispatch_release(sema); + + authResult.success = success; + authResult.error_code = errorCode; + if (errorMessage != nil) { + authResult.error_msg = strdup([errorMessage UTF8String]); // Copy error message to C string + } else { + authResult.error_msg = NULL; + } + + return authResult; +} +*/ +import "C" +import ( + "fmt" + "unsafe" +) + +func detect(reason string) (bool, error) { + reasonStr := C.CString(reason) + defer C.free(unsafe.Pointer(reasonStr)) + + result := C.Authenticate(reasonStr) + + // Convert C error message to Go string + if result.error_msg != nil { + defer C.free(unsafe.Pointer(result.error_msg)) + } + errorMessage := C.GoString(result.error_msg) + + // Return success or failure, with an error if applicable + if result.success { + return true, nil + } + + return false, fmt.Errorf("authentication failed: %d %s", int(result.error_code), errorMessage) +} diff --git a/ee/presence/presence_darwin_test.go b/ee/presence/presence_darwin_test.go new file mode 100644 index 000000000..fad50ab5f --- /dev/null +++ b/ee/presence/presence_darwin_test.go @@ -0,0 +1,48 @@ +package presence + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testPresenceEnvVar = "launcher_test_presence" + +// Since there is no way to test user presence in a CI / automated fashion, +// these test are expected to be run manually via cmd line when needed. + +// To test this run +// +// launcher_test_presence=true go test ./ee/presence/ -run Test_biometricDetectSuccess +// +// then successfully auth with the pop up +func Test_biometricDetectSuccess(t *testing.T) { + t.Parallel() + + if os.Getenv(testPresenceEnvVar) == "" { + t.Skip("Skipping test_biometricDetectSuccess") + } + + success, err := detect("IS TRYING TO TEST SUCCESS, PLEASE AUTHENTICATE") + require.NoError(t, err, "should not get an error on successful detect") + assert.True(t, success, "should be successful") +} + +// To test this run +// +// launcher_test_presence=true go test ./ee/presence/ -run Test_biometricDetectCancel +// +// then cancel the biometric auth that pops up +func Test_biometricDetectCancel(t *testing.T) { + t.Parallel() + + if os.Getenv(testPresenceEnvVar) == "" { + t.Skip("Skipping test_biometricDetectCancel") + } + + success, err := detect("IS TRYING TO TEST CANCEL, PLEASE PRESS CANCEL") + require.Error(t, err, "should get an error on failed detect") + assert.False(t, success, "should not be successful") +} From 97f42879efb8e00c54ad3e1cdb4261ef78b845c6 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Fri, 13 Sep 2024 13:52:25 -0700 Subject: [PATCH 02/23] naming --- ee/presence/presence_darwin_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ee/presence/presence_darwin_test.go b/ee/presence/presence_darwin_test.go index fad50ab5f..833f4f673 100644 --- a/ee/presence/presence_darwin_test.go +++ b/ee/presence/presence_darwin_test.go @@ -15,10 +15,10 @@ const testPresenceEnvVar = "launcher_test_presence" // To test this run // -// launcher_test_presence=true go test ./ee/presence/ -run Test_biometricDetectSuccess +// launcher_test_presence=true go test ./ee/presence/ -run Test_detectSuccess // // then successfully auth with the pop up -func Test_biometricDetectSuccess(t *testing.T) { +func Test_detectSuccess(t *testing.T) { t.Parallel() if os.Getenv(testPresenceEnvVar) == "" { @@ -32,10 +32,10 @@ func Test_biometricDetectSuccess(t *testing.T) { // To test this run // -// launcher_test_presence=true go test ./ee/presence/ -run Test_biometricDetectCancel +// launcher_test_presence=true go test ./ee/presence/ -run Test_detectCancel // // then cancel the biometric auth that pops up -func Test_biometricDetectCancel(t *testing.T) { +func Test_detectCancel(t *testing.T) { t.Parallel() if os.Getenv(testPresenceEnvVar) == "" { From 566d5bd963addb556db102868af8057360d36887 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Tue, 17 Sep 2024 14:24:34 -0700 Subject: [PATCH 03/23] rough exec implementation of presence detection --- cmd/launcher/detect_presence.go | 48 ++++++++ cmd/launcher/main.go | 2 + ee/localserver/server.go | 33 ++++- ee/presencedetection/presencedetection.go | 116 ++++++++++++++++++ .../presencedetection_darwin.go} | 7 +- .../presencedetection_darwin_test.go} | 12 +- .../presencedetection_other.go | 11 ++ 7 files changed, 218 insertions(+), 11 deletions(-) create mode 100644 cmd/launcher/detect_presence.go create mode 100644 ee/presencedetection/presencedetection.go rename ee/{presence/presence_darwin.go => presencedetection/presencedetection_darwin.go} (97%) rename ee/{presence/presence_darwin_test.go => presencedetection/presencedetection_darwin_test.go} (72%) create mode 100644 ee/presencedetection/presencedetection_other.go diff --git a/cmd/launcher/detect_presence.go b/cmd/launcher/detect_presence.go new file mode 100644 index 000000000..2ab17f1d7 --- /dev/null +++ b/cmd/launcher/detect_presence.go @@ -0,0 +1,48 @@ +package main + +import ( + "encoding/base64" + "encoding/json" + "errors" + "os" + "runtime" + + "github.com/kolide/launcher/ee/presencedetection" + "github.com/kolide/launcher/pkg/log/multislogger" +) + +func runDetectPresence(_ *multislogger.MultiSlogger, args []string) error { + reason := "" + + if len(args) != 0 { + reason = args[0] + } + + if reason == "" && runtime.GOOS == "darwin" { + return errors.New("reason is required on darwin") + } + + success, err := presencedetection.Detect(reason) + response := presencedetection.PresenceDetectionResponse{ + Success: success, + } + if err != nil { + response.Error = err.Error() + } + + // serialize response to JSON + responseJSON, err := json.Marshal(response) + if err != nil { + return err + } + + // b64 enode response + responseB64 := base64.StdEncoding.EncodeToString(responseJSON) + + // write response to stdout + if _, err := os.Stdout.Write([]byte(responseB64)); err != nil { + return err + } + + return nil +} diff --git a/cmd/launcher/main.go b/cmd/launcher/main.go index a8eaf0818..21836ab7a 100644 --- a/cmd/launcher/main.go +++ b/cmd/launcher/main.go @@ -196,6 +196,8 @@ func runSubcommands(systemMultiSlogger *multislogger.MultiSlogger) error { run = runSecureEnclave case "watchdog": // note: this is currently only implemented for windows run = watchdog.RunWatchdogService + case "detect-presence": + run = runDetectPresence default: return fmt.Errorf("unknown subcommand %s", os.Args[1]) } diff --git a/ee/localserver/server.go b/ee/localserver/server.go index 06f2b8fda..2c74fef93 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -19,6 +19,7 @@ import ( "github.com/kolide/krypto/pkg/echelper" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/agent/types" + "github.com/kolide/launcher/ee/presencedetection" "github.com/kolide/launcher/pkg/osquery" "github.com/kolide/launcher/pkg/traces" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" @@ -56,6 +57,8 @@ type localServer struct { serverKey *rsa.PublicKey serverEcKey *ecdsa.PublicKey + + presenceDetector presencedetection.PresenceDetector } const ( @@ -121,9 +124,15 @@ func New(ctx context.Context, k types.Knapsack) (*localServer, error) { // mux.Handle("/acceleratecontrol", ls.requestAccelerateControlHandler()) srv := &http.Server{ - Handler: otelhttp.NewHandler(ls.requestLoggingHandler(ls.preflightCorsHandler(ls.rateLimitHandler(mux))), "localserver", otelhttp.WithSpanNameFormatter(func(operation string, r *http.Request) string { - return r.URL.Path - })), + Handler: otelhttp.NewHandler( + ls.requestLoggingHandler( + ls.preflightCorsHandler( + ls.rateLimitHandler( + ls.presenceDetectionHandler(mux), + ), + )), "localserver", otelhttp.WithSpanNameFormatter(func(operation string, r *http.Request) string { + return r.URL.Path + })), ReadTimeout: 500 * time.Millisecond, ReadHeaderTimeout: 50 * time.Millisecond, // WriteTimeout very high due to retry logic in the scheduledquery endpoint @@ -393,3 +402,21 @@ func (ls *localServer) rateLimitHandler(next http.Handler) http.Handler { next.ServeHTTP(w, r) }) } + +func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + + success, err := ls.presenceDetector.DetectForConsoleUser("sign you into a thing...", 1*time.Minute) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + if !success { + http.Error(w, "presence detection failed", http.StatusInternalServerError) + return + } + + next.ServeHTTP(w, r) + }) +} diff --git a/ee/presencedetection/presencedetection.go b/ee/presencedetection/presencedetection.go new file mode 100644 index 000000000..1565d2e5e --- /dev/null +++ b/ee/presencedetection/presencedetection.go @@ -0,0 +1,116 @@ +package presencedetection + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "os" + "os/exec" + "os/user" + "strings" + "sync" + "time" + + "github.com/kolide/launcher/ee/consoleuser" +) + +type PresenceDetectionResponse struct { + Success bool `json:"success"` + Error string `json:"error,omitempty"` +} + +type PresenceDetector struct { + lastDetectionUTC time.Time + mutext sync.Mutex +} + +func (pd *PresenceDetector) DetectForConsoleUser(reason string, detectionInterval time.Duration) (bool, error) { + pd.mutext.Lock() + defer pd.mutext.Unlock() + + // Check if the last detection was within the detection interval + if time.Since(pd.lastDetectionUTC) < detectionInterval { + return true, nil + } + + executablePath, err := os.Executable() + if err != nil { + return false, fmt.Errorf("could not get executable path: %w", err) + } + + consoleUserUids, err := consoleuser.CurrentUids(context.TODO()) + if err != nil { + return false, fmt.Errorf("could not get console user: %w", err) + } + + if len(consoleUserUids) == 0 { + return false, errors.New("no console user found") + } + + runningUserUid := consoleUserUids[0] + + // Ensure that we handle a non-root current user appropriately + currentUser, err := user.Current() + if err != nil { + return false, fmt.Errorf("getting current user: %w", err) + } + + runningUser, err := user.LookupId(runningUserUid) + if err != nil || runningUser == nil { + return false, fmt.Errorf("looking up user with uid %s: %w", runningUserUid, err) + } + + cmd := exec.Command(executablePath, "detect-presence", reason) //nolint:forbidigo // We trust that the launcher executable path is correct, so we don't need to use allowedcmd + + // Update command so that we're prepending `launchctl asuser $UID sudo --preserve-env -u $runningUser` to the launcher desktop command. + // We need to run with `launchctl asuser` in order to get the user context, which is required to be able to send notifications. + // We need `sudo -u $runningUser` to set the UID on the command correctly -- necessary for, among other things, correctly observing + // light vs dark mode. + // We need --preserve-env for sudo in order to avoid clearing SOCKET_PATH, AUTHTOKEN, etc that are necessary for the desktop + // process to run. + cmd.Path = "/bin/launchctl" + updatedCmdArgs := append([]string{"/bin/launchctl", "asuser", runningUserUid, "sudo", "--preserve-env", "-u", runningUser.Username}, cmd.Args...) + cmd.Args = updatedCmdArgs + + if currentUser.Uid != "0" && currentUser.Uid != runningUserUid { + // if the user is running for another user, we have an error because we can't set credentials + return false, fmt.Errorf("current user %s is not root and does not match running user, can't start process for other user %s", currentUser.Uid, runningUserUid) + } + + out, err := cmd.CombinedOutput() + if err != nil { + return false, fmt.Errorf("could not run command: %w", err) + } + + outStr := string(out) + + // get last line of outstr + lastLine := "" + for _, line := range strings.Split(outStr, "\n") { + if line != "" { + lastLine = line + } + } + + outDecoded, err := base64.StdEncoding.DecodeString(lastLine) + if err != nil { + return false, fmt.Errorf("could not decode output: %w", err) + } + + response := PresenceDetectionResponse{} + if err := json.Unmarshal(outDecoded, &response); err != nil { + return false, fmt.Errorf("could not unmarshal response: %w", err) + } + + if response.Success { + pd.lastDetectionUTC = time.Now().UTC() + } + + if response.Error != "" { + return response.Success, errors.New(response.Error) + } + + return response.Success, nil +} diff --git a/ee/presence/presence_darwin.go b/ee/presencedetection/presencedetection_darwin.go similarity index 97% rename from ee/presence/presence_darwin.go rename to ee/presencedetection/presencedetection_darwin.go index 1264d4c52..8f7ebe79a 100644 --- a/ee/presence/presence_darwin.go +++ b/ee/presencedetection/presencedetection_darwin.go @@ -1,4 +1,7 @@ -package presence +//go:build darwin +// +build darwin + +package presencedetection /* #cgo CFLAGS: -x objective-c -fmodules -fblocks @@ -81,7 +84,7 @@ import ( "unsafe" ) -func detect(reason string) (bool, error) { +func Detect(reason string) (bool, error) { reasonStr := C.CString(reason) defer C.free(unsafe.Pointer(reasonStr)) diff --git a/ee/presence/presence_darwin_test.go b/ee/presencedetection/presencedetection_darwin_test.go similarity index 72% rename from ee/presence/presence_darwin_test.go rename to ee/presencedetection/presencedetection_darwin_test.go index 833f4f673..4241316dd 100644 --- a/ee/presence/presence_darwin_test.go +++ b/ee/presencedetection/presencedetection_darwin_test.go @@ -1,4 +1,4 @@ -package presence +package presencedetection import ( "os" @@ -15,24 +15,24 @@ const testPresenceEnvVar = "launcher_test_presence" // To test this run // -// launcher_test_presence=true go test ./ee/presence/ -run Test_detectSuccess +// launcher_test_presence=true go test ./ee/presencedetection/ -run Test_detectSuccess // // then successfully auth with the pop up func Test_detectSuccess(t *testing.T) { t.Parallel() if os.Getenv(testPresenceEnvVar) == "" { - t.Skip("Skipping test_biometricDetectSuccess") + t.Skip("Skipping Test_detectSuccess") } - success, err := detect("IS TRYING TO TEST SUCCESS, PLEASE AUTHENTICATE") + success, err := Detect("IS TRYING TO TEST SUCCESS, PLEASE AUTHENTICATE") require.NoError(t, err, "should not get an error on successful detect") assert.True(t, success, "should be successful") } // To test this run // -// launcher_test_presence=true go test ./ee/presence/ -run Test_detectCancel +// launcher_test_presence=true go test ./ee/presencedetection/ -run Test_detectCancel // // then cancel the biometric auth that pops up func Test_detectCancel(t *testing.T) { @@ -42,7 +42,7 @@ func Test_detectCancel(t *testing.T) { t.Skip("Skipping test_biometricDetectCancel") } - success, err := detect("IS TRYING TO TEST CANCEL, PLEASE PRESS CANCEL") + success, err := Detect("IS TRYING TO TEST CANCEL, PLEASE PRESS CANCEL") require.Error(t, err, "should get an error on failed detect") assert.False(t, success, "should not be successful") } diff --git a/ee/presencedetection/presencedetection_other.go b/ee/presencedetection/presencedetection_other.go new file mode 100644 index 000000000..bcb742bf0 --- /dev/null +++ b/ee/presencedetection/presencedetection_other.go @@ -0,0 +1,11 @@ +//go:build !darwin +// +build !darwin + +package presencedetection + +import "errors" + +func Detect(reason string) (bool, error) { + // Implement detection logic for non-Darwin platforms + return false, errors.New("detection not implemented for this platform") +} From ecd9d3ecfe2a8ca13368c5968315b5ecec8f632e Mon Sep 17 00:00:00 2001 From: James Pickett Date: Wed, 18 Sep 2024 15:00:10 -0700 Subject: [PATCH 04/23] add headers to v2CmdRequestType, act on headers in presence detection middleware --- ee/localserver/krypto-ec-middleware.go | 16 ++++++++++++---- ee/localserver/server.go | 22 +++++++++++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index c7f28a0d6..6a3e0b0a2 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -25,15 +25,17 @@ import ( ) const ( - timestampValidityRange = 150 - kolideKryptoEccHeader20230130Value = "2023-01-30" - kolideKryptoHeaderKey = "X-Kolide-Krypto" - kolideSessionIdHeaderKey = "X-Kolide-Session" + timestampValidityRange = 150 + kolideKryptoEccHeader20230130Value = "2023-01-30" + kolideKryptoHeaderKey = "X-Kolide-Krypto" + kolideSessionIdHeaderKey = "X-Kolide-Session" + kolidePresenceDetectionIntervalSecondsKey = "X-Kolide-Presence-Detection-Interval" ) type v2CmdRequestType struct { Path string Body []byte + Headers map[string][]string CallbackUrl string CallbackHeaders map[string][]string AllowedOrigins []string @@ -285,6 +287,12 @@ func (e *kryptoEcMiddleware) Wrap(next http.Handler) http.Handler { }, } + for h, vals := range cmdReq.Headers { + for _, v := range vals { + newReq.Header.Add(h, v) + } + } + newReq.Header.Set("Origin", r.Header.Get("Origin")) newReq.Header.Set("Referer", r.Header.Get("Referer")) diff --git a/ee/localserver/server.go b/ee/localserver/server.go index 2c74fef93..801b59e12 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -12,6 +12,7 @@ import ( "log/slog" "net" "net/http" + "strconv" "strings" "time" @@ -406,14 +407,29 @@ func (ls *localServer) rateLimitHandler(next http.Handler) http.Handler { func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - success, err := ls.presenceDetector.DetectForConsoleUser("sign you into a thing...", 1*time.Minute) + // can test this by adding an unauthed endpoint to the mux and running, for example: + // curl -H "X-Kolide-Presence-Detection-Interval: 0" localhost:12519/id + detectionIntervalSecondsStr := r.Header.Get(kolidePresenceDetectionIntervalSecondsKey) + + // no presence detection requested + if detectionIntervalSecondsStr == "" { + next.ServeHTTP(w, r) + } + + detectionIntervalSeconds, err := strconv.Atoi(detectionIntervalSecondsStr) + if err != nil { + http.Error(w, err.Error(), http.StatusUnauthorized) + return + } + + success, err := ls.presenceDetector.DetectForConsoleUser("detect user presence", time.Duration(detectionIntervalSeconds)*time.Second) if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, err.Error(), http.StatusUnauthorized) return } if !success { - http.Error(w, "presence detection failed", http.StatusInternalServerError) + http.Error(w, "presence detection failed", http.StatusUnauthorized) return } From d7efbabaa7647f10ef2477f0010c5f173abea742 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 19 Sep 2024 14:46:49 -0700 Subject: [PATCH 05/23] dont show desktop until requested by runner --- cmd/launcher/desktop.go | 7 ++- ee/desktop/runner/runner.go | 81 ++++++++++++++++++--------- ee/desktop/user/client/client.go | 4 ++ ee/desktop/user/client/client_test.go | 2 +- ee/desktop/user/server/server.go | 30 ++++++++-- ee/desktop/user/server/server_test.go | 2 +- 6 files changed, 89 insertions(+), 37 deletions(-) diff --git a/cmd/launcher/desktop.go b/cmd/launcher/desktop.go index a397fadb0..ea13e515e 100644 --- a/cmd/launcher/desktop.go +++ b/cmd/launcher/desktop.go @@ -130,7 +130,9 @@ func runDesktop(_ *multislogger.MultiSlogger, args []string) error { }, func(error) {}) shutdownChan := make(chan struct{}) - server, err := userserver.New(slogger, *flUserServerAuthToken, *flUserServerSocketPath, shutdownChan, notifier) + showDesktopChan := make(chan struct{}) + + server, err := userserver.New(slogger, *flUserServerAuthToken, *flUserServerSocketPath, shutdownChan, showDesktopChan, notifier) if err != nil { return err } @@ -182,9 +184,10 @@ func runDesktop(_ *multislogger.MultiSlogger, args []string) error { } }() + // block until a send on showDesktopChan + <-showDesktopChan // blocks until shutdown called m.Init() - return nil } diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index db02aaf98..fcadd707b 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -118,9 +118,6 @@ type DesktopUsersProcessesRunner struct { // usersFilesRoot is the launcher root dir with will be the parent dir // for kolide desktop files on a per user basis usersFilesRoot string - // processSpawningEnabled controls whether or not desktop user processes are automatically spawned - // This effectively represents whether or not the launcher desktop GUI is enabled or not - processSpawningEnabled bool // knapsack is the almighty sack of knaps knapsack types.Knapsack // runnerServer is a local server that desktop processes call to monitor parent @@ -155,17 +152,16 @@ func (pr processRecord) String() string { // New creates and returns a new DesktopUsersProcessesRunner runner and initializes all required fields func New(k types.Knapsack, messenger runnerserver.Messenger, opts ...desktopUsersProcessesRunnerOption) (*DesktopUsersProcessesRunner, error) { runner := &DesktopUsersProcessesRunner{ - interrupt: make(chan struct{}), - uidProcs: make(map[string]processRecord), - updateInterval: k.DesktopUpdateInterval(), - menuRefreshInterval: k.DesktopMenuRefreshInterval(), - procsWg: &sync.WaitGroup{}, - interruptTimeout: time.Second * 5, - hostname: k.KolideServerURL(), - usersFilesRoot: agent.TempPath("kolide-desktop"), - processSpawningEnabled: k.DesktopEnabled(), - knapsack: k, - cachedMenuData: newMenuItemCache(), + interrupt: make(chan struct{}), + uidProcs: make(map[string]processRecord), + updateInterval: k.DesktopUpdateInterval(), + menuRefreshInterval: k.DesktopMenuRefreshInterval(), + procsWg: &sync.WaitGroup{}, + interruptTimeout: time.Second * 5, + hostname: k.KolideServerURL(), + usersFilesRoot: agent.TempPath("kolide-desktop"), + knapsack: k, + cachedMenuData: newMenuItemCache(), } runner.slogger = k.Slogger().With("component", "desktop_runner") @@ -452,12 +448,35 @@ func (r *DesktopUsersProcessesRunner) Update(data io.Reader) error { } func (r *DesktopUsersProcessesRunner) FlagsChanged(flagKeys ...keys.FlagKey) { - if slices.Contains(flagKeys, keys.DesktopEnabled) { - r.processSpawningEnabled = r.knapsack.DesktopEnabled() - r.slogger.Log(context.TODO(), slog.LevelDebug, - "runner processSpawningEnabled set by control server", - "process_spawning_enabled", r.processSpawningEnabled, - ) + if !slices.Contains(flagKeys, keys.DesktopEnabled) { + return + } + + r.slogger.Log(context.TODO(), slog.LevelDebug, + "desktop enabled set by control server", + "process_spawning_enabled", r.knapsack.DesktopEnabled(), + ) + + if !r.knapsack.DesktopEnabled() { + // there is no way to "hide" the menu, so we will just kill any existing processes + // they will respawn in "silent" mode + r.killDesktopProcesses(context.TODO()) + return + } + + // DesktopEnabled() == true + // Tell any running desktop user processes that they should show the menu + for uid, proc := range r.uidProcs { + client := client.New(r.userServerAuthToken, proc.socketPath) + if err := client.ShowDesktop(); err != nil { + r.slogger.Log(context.TODO(), slog.LevelError, + "sending refresh command to user desktop process", + "uid", uid, + "pid", proc.Process.Pid, + "path", proc.path, + "err", err, + ) + } } } @@ -483,6 +502,10 @@ func (r *DesktopUsersProcessesRunner) writeSharedFile(path string, data []byte) // refreshMenu updates the menu file and tells desktop processes to refresh their menus func (r *DesktopUsersProcessesRunner) refreshMenu() { + if !r.knapsack.DesktopEnabled() { + return + } + if err := r.generateMenuFile(); err != nil { if r.knapsack.DebugServerData() { r.slogger.Log(context.TODO(), slog.LevelError, @@ -502,8 +525,18 @@ func (r *DesktopUsersProcessesRunner) refreshMenu() { // Tell any running desktop user processes that they should refresh the latest menu data for uid, proc := range r.uidProcs { client := client.New(r.userServerAuthToken, proc.socketPath) - if err := client.Refresh(); err != nil { + if err := client.ShowDesktop(); err != nil { + r.slogger.Log(context.TODO(), slog.LevelError, + "sending refresh command to user desktop process", + "uid", uid, + "pid", proc.Process.Pid, + "path", proc.path, + "err", err, + ) + } + + if err := client.Refresh(); err != nil { r.slogger.Log(context.TODO(), slog.LevelError, "sending refresh command to user desktop process", "uid", uid, @@ -601,12 +634,6 @@ func (r *DesktopUsersProcessesRunner) runConsoleUserDesktop() error { return nil } - if !r.processSpawningEnabled { - // Desktop is disabled, kill any existing desktop user processes - r.killDesktopProcesses(context.Background()) - return nil - } - executablePath, err := r.determineExecutablePath() if err != nil { return fmt.Errorf("determining executable path: %w", err) diff --git a/ee/desktop/user/client/client.go b/ee/desktop/user/client/client.go index 8fe1641b6..ab707df96 100644 --- a/ee/desktop/user/client/client.go +++ b/ee/desktop/user/client/client.go @@ -55,6 +55,10 @@ func (c *client) Refresh() error { return c.get("refresh") } +func (c *client) ShowDesktop() error { + return c.get("show") +} + func (c *client) Notify(n notify.Notification) error { notificationToSend := notify.Notification{ Title: n.Title, diff --git a/ee/desktop/user/client/client_test.go b/ee/desktop/user/client/client_test.go index 97884c9d1..f000a2e2c 100644 --- a/ee/desktop/user/client/client_test.go +++ b/ee/desktop/user/client/client_test.go @@ -42,7 +42,7 @@ func TestClient_GetAndShutdown(t *testing.T) { socketPath := testSocketPath(t) shutdownChan := make(chan struct{}) - server, err := server.New(multislogger.NewNopLogger(), validAuthToken, socketPath, shutdownChan, nil) + server, err := server.New(multislogger.NewNopLogger(), validAuthToken, socketPath, shutdownChan, make(chan<- struct{}), nil) require.NoError(t, err) go func() { diff --git a/ee/desktop/user/server/server.go b/ee/desktop/user/server/server.go index 87bd16636..3be45ebe4 100644 --- a/ee/desktop/user/server/server.go +++ b/ee/desktop/user/server/server.go @@ -13,6 +13,7 @@ import ( "os" "runtime" "strings" + "sync" "time" "github.com/kolide/launcher/ee/desktop/user/notify" @@ -30,19 +31,26 @@ type UserServer struct { server *http.Server listener net.Listener shutdownChan chan<- struct{} + showDesktopChan chan<- struct{} authToken string socketPath string notifier notificationSender refreshListeners []func() } -func New(slogger *slog.Logger, authToken string, socketPath string, shutdownChan chan<- struct{}, notifier notificationSender) (*UserServer, error) { +func New(slogger *slog.Logger, + authToken string, + socketPath string, + shutdownChan chan<- struct{}, + showDesktopChan chan<- struct{}, + notifier notificationSender) (*UserServer, error) { userServer := &UserServer{ - shutdownChan: shutdownChan, - authToken: authToken, - slogger: slogger.With("component", "desktop_server"), - socketPath: socketPath, - notifier: notifier, + shutdownChan: shutdownChan, + showDesktopChan: showDesktopChan, + authToken: authToken, + slogger: slogger.With("component", "desktop_server"), + socketPath: socketPath, + notifier: notifier, } authedMux := http.NewServeMux() @@ -50,6 +58,7 @@ func New(slogger *slog.Logger, authToken string, socketPath string, shutdownChan authedMux.HandleFunc("/ping", userServer.pingHandler) authedMux.HandleFunc("/notification", userServer.notificationHandler) authedMux.HandleFunc("/refresh", userServer.refreshHandler) + authedMux.HandleFunc("/show", userServer.showDesktop) userServer.server = &http.Server{ Handler: userServer.authMiddleware(authedMux), @@ -152,6 +161,15 @@ func (s *UserServer) notificationHandler(w http.ResponseWriter, req *http.Reques w.WriteHeader(http.StatusOK) } +func (s *UserServer) showDesktop(w http.ResponseWriter, req *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + + sync.OnceFunc(func() { + s.showDesktopChan <- struct{}{} + })() +} + func (s *UserServer) refreshHandler(w http.ResponseWriter, req *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) diff --git a/ee/desktop/user/server/server_test.go b/ee/desktop/user/server/server_test.go index 964c4639a..6ec884560 100644 --- a/ee/desktop/user/server/server_test.go +++ b/ee/desktop/user/server/server_test.go @@ -121,7 +121,7 @@ func testServer(t *testing.T, authHeader, socketPath string, logBytes *bytes.Buf Level: slog.LevelDebug, })) - server, err := New(slogger, authHeader, socketPath, shutdownChan, nil) + server, err := New(slogger, authHeader, socketPath, shutdownChan, make(chan<- struct{}), nil) require.NoError(t, err) return server, shutdownChan } From 4b99368c20512d0933c48074e4bf0e80fb766e12 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Fri, 20 Sep 2024 11:23:23 -0700 Subject: [PATCH 06/23] do presence detection through desktop server --- cmd/launcher/detect_presence.go | 48 ------------ cmd/launcher/main.go | 2 - ee/desktop/runner/runner.go | 48 ++++++++---- ee/desktop/user/client/client.go | 29 ++++++++ ee/desktop/user/server/server.go | 89 ++++++++++++++++++----- ee/localserver/krypto-ec-middleware.go | 10 +-- ee/localserver/server.go | 18 +++-- ee/presencedetection/presencedetection.go | 88 ++-------------------- 8 files changed, 155 insertions(+), 177 deletions(-) delete mode 100644 cmd/launcher/detect_presence.go diff --git a/cmd/launcher/detect_presence.go b/cmd/launcher/detect_presence.go deleted file mode 100644 index 2ab17f1d7..000000000 --- a/cmd/launcher/detect_presence.go +++ /dev/null @@ -1,48 +0,0 @@ -package main - -import ( - "encoding/base64" - "encoding/json" - "errors" - "os" - "runtime" - - "github.com/kolide/launcher/ee/presencedetection" - "github.com/kolide/launcher/pkg/log/multislogger" -) - -func runDetectPresence(_ *multislogger.MultiSlogger, args []string) error { - reason := "" - - if len(args) != 0 { - reason = args[0] - } - - if reason == "" && runtime.GOOS == "darwin" { - return errors.New("reason is required on darwin") - } - - success, err := presencedetection.Detect(reason) - response := presencedetection.PresenceDetectionResponse{ - Success: success, - } - if err != nil { - response.Error = err.Error() - } - - // serialize response to JSON - responseJSON, err := json.Marshal(response) - if err != nil { - return err - } - - // b64 enode response - responseB64 := base64.StdEncoding.EncodeToString(responseJSON) - - // write response to stdout - if _, err := os.Stdout.Write([]byte(responseB64)); err != nil { - return err - } - - return nil -} diff --git a/cmd/launcher/main.go b/cmd/launcher/main.go index 21836ab7a..a8eaf0818 100644 --- a/cmd/launcher/main.go +++ b/cmd/launcher/main.go @@ -196,8 +196,6 @@ func runSubcommands(systemMultiSlogger *multislogger.MultiSlogger) error { run = runSecureEnclave case "watchdog": // note: this is currently only implemented for windows run = watchdog.RunWatchdogService - case "detect-presence": - run = runDetectPresence default: return fmt.Errorf("unknown subcommand %s", os.Args[1]) } diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index fcadd707b..6473ef858 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -88,6 +88,32 @@ func InstanceDesktopProcessRecords() map[string]processRecord { return instance.uidProcs } +func InstanceDetectPresence(reason string, interval time.Duration) (bool, error) { + if instance == nil { + return false, errors.New("no instance of DesktopUsersProcessesRunner") + } + + if instance.uidProcs == nil || len(instance.uidProcs) == 0 { + return false, errors.New("no desktop processes running") + } + + var lastErr error + for _, proc := range instance.uidProcs { + client := client.New(instance.userServerAuthToken, proc.socketPath) + success, err := client.DetectPresence(reason, interval) + + // not sure how to handle the possiblity of multiple users + // so just return the first success + if success { + return success, err + } + + lastErr = err + } + + return false, fmt.Errorf("no desktop processes detected presence, last error: %w", lastErr) +} + // DesktopUsersProcessesRunner creates a launcher desktop process each time it detects // a new console (GUI) user. If the current console user's desktop process dies, it // will create a new one. @@ -522,20 +548,10 @@ func (r *DesktopUsersProcessesRunner) refreshMenu() { return } - // Tell any running desktop user processes that they should refresh the latest menu data + // Tell any running desktop user processes that they should show menu and refresh the latest menu data for uid, proc := range r.uidProcs { client := client.New(r.userServerAuthToken, proc.socketPath) - if err := client.ShowDesktop(); err != nil { - r.slogger.Log(context.TODO(), slog.LevelError, - "sending refresh command to user desktop process", - "uid", uid, - "pid", proc.Process.Pid, - "path", proc.path, - "err", err, - ) - } - if err := client.Refresh(); err != nil { r.slogger.Log(context.TODO(), slog.LevelError, "sending refresh command to user desktop process", @@ -696,13 +712,19 @@ func (r *DesktopUsersProcessesRunner) spawnForUser(ctx context.Context, uid stri r.waitOnProcessAsync(uid, cmd.Process) client := client.New(r.userServerAuthToken, socketPath) - if err := backoff.WaitFor(client.Ping, 10*time.Second, 1*time.Second); err != nil { + + pingFunc := client.Ping + if r.knapsack.DesktopEnabled() { + pingFunc = client.ShowDesktop + } + + if err := backoff.WaitFor(pingFunc, 10*time.Second, 1*time.Second); err != nil { // unregister proc from desktop server so server will not respond to its requests r.runnerServer.DeRegisterClient(uid) if err := cmd.Process.Kill(); err != nil { r.slogger.Log(ctx, slog.LevelError, - "killing user desktop process after startup ping failed", + "killing user desktop process after startup ping / show desktop failed", "uid", uid, "pid", cmd.Process.Pid, "path", cmd.Path, diff --git a/ee/desktop/user/client/client.go b/ee/desktop/user/client/client.go index ab707df96..6c82358ae 100644 --- a/ee/desktop/user/client/client.go +++ b/ee/desktop/user/client/client.go @@ -4,11 +4,14 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "net/http" + "net/url" "time" "github.com/kolide/launcher/ee/desktop/user/notify" + "github.com/kolide/launcher/ee/desktop/user/server" ) type transport struct { @@ -59,6 +62,32 @@ func (c *client) ShowDesktop() error { return c.get("show") } +func (c *client) DetectPresence(reason string, interval time.Duration) (bool, error) { + encodedReason := url.QueryEscape(reason) + encodedInterval := url.QueryEscape(interval.String()) + + resp, requestErr := c.base.Get(fmt.Sprintf("http://unix/detect_presence?reason=%s&interval=%s", encodedReason, encodedInterval)) + if requestErr != nil { + return false, fmt.Errorf("getting presence: %w", requestErr) + } + + var response server.DetectPresenceResponse + if resp.Body != nil { + defer resp.Body.Close() + + if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { + return false, fmt.Errorf("decoding response: %w", err) + } + } + + var err error + if response.Error != "" { + err = errors.New(response.Error) + } + + return response.Success, err +} + func (c *client) Notify(n notify.Notification) error { notificationToSend := notify.Notification{ Title: n.Title, diff --git a/ee/desktop/user/server/server.go b/ee/desktop/user/server/server.go index 3be45ebe4..8efa2c6e6 100644 --- a/ee/desktop/user/server/server.go +++ b/ee/desktop/user/server/server.go @@ -17,6 +17,7 @@ import ( "time" "github.com/kolide/launcher/ee/desktop/user/notify" + "github.com/kolide/launcher/ee/presencedetection" "github.com/kolide/launcher/pkg/backoff" ) @@ -27,15 +28,16 @@ type notificationSender interface { // UserServer provides IPC for the root desktop runner to communicate with the user desktop processes. // It allows the runner process to send notficaitons and commands to the desktop processes. type UserServer struct { - slogger *slog.Logger - server *http.Server - listener net.Listener - shutdownChan chan<- struct{} - showDesktopChan chan<- struct{} - authToken string - socketPath string - notifier notificationSender - refreshListeners []func() + slogger *slog.Logger + server *http.Server + listener net.Listener + shutdownChan chan<- struct{} + authToken string + socketPath string + notifier notificationSender + refreshListeners []func() + presenceDetector presencedetection.PresenceDetector + showDesktopOnceFunc func() } func New(slogger *slog.Logger, @@ -45,12 +47,14 @@ func New(slogger *slog.Logger, showDesktopChan chan<- struct{}, notifier notificationSender) (*UserServer, error) { userServer := &UserServer{ - shutdownChan: shutdownChan, - showDesktopChan: showDesktopChan, - authToken: authToken, - slogger: slogger.With("component", "desktop_server"), - socketPath: socketPath, - notifier: notifier, + shutdownChan: shutdownChan, + authToken: authToken, + slogger: slogger.With("component", "desktop_server"), + socketPath: socketPath, + notifier: notifier, + showDesktopOnceFunc: sync.OnceFunc(func() { + showDesktopChan <- struct{}{} + }), } authedMux := http.NewServeMux() @@ -59,6 +63,7 @@ func New(slogger *slog.Logger, authedMux.HandleFunc("/notification", userServer.notificationHandler) authedMux.HandleFunc("/refresh", userServer.refreshHandler) authedMux.HandleFunc("/show", userServer.showDesktop) + authedMux.HandleFunc("/detect_presence", userServer.detectPresence) userServer.server = &http.Server{ Handler: userServer.authMiddleware(authedMux), @@ -165,9 +170,57 @@ func (s *UserServer) showDesktop(w http.ResponseWriter, req *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - sync.OnceFunc(func() { - s.showDesktopChan <- struct{}{} - })() + s.showDesktopOnceFunc() +} + +type DetectPresenceResponse struct { + Success bool `json:"success"` + Error string `json:"error,omitempty"` +} + +func (s *UserServer) detectPresence(w http.ResponseWriter, req *http.Request) { + // get reason url param from req + reason := req.URL.Query().Get("reason") + + if reason == "" { + http.Error(w, "reason is required", http.StatusBadRequest) + return + } + + // get intervalString from url param + intervalString := req.URL.Query().Get("interval") + if intervalString == "" { + http.Error(w, "interval is required", http.StatusBadRequest) + return + } + + interval, err := time.ParseDuration(intervalString) + if err != nil { + http.Error(w, "interval is not a valid duration", http.StatusBadRequest) + return + } + + // detect presence + success, err := s.presenceDetector.DetectPresence(reason, interval) + response := DetectPresenceResponse{ + Success: success, + } + + if err != nil { + response.Error = err.Error() + } + + // convert response to json + responseBytes, err := json.Marshal(response) + if err != nil { + http.Error(w, "could not marshal response", http.StatusInternalServerError) + return + } + + // write response + w.Header().Set("Content-Type", "application/json") + w.Write(responseBytes) + w.WriteHeader(http.StatusOK) } func (s *UserServer) refreshHandler(w http.ResponseWriter, req *http.Request) { diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index 6a3e0b0a2..12bfea8ad 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -25,11 +25,11 @@ import ( ) const ( - timestampValidityRange = 150 - kolideKryptoEccHeader20230130Value = "2023-01-30" - kolideKryptoHeaderKey = "X-Kolide-Krypto" - kolideSessionIdHeaderKey = "X-Kolide-Session" - kolidePresenceDetectionIntervalSecondsKey = "X-Kolide-Presence-Detection-Interval" + timestampValidityRange = 150 + kolideKryptoEccHeader20230130Value = "2023-01-30" + kolideKryptoHeaderKey = "X-Kolide-Krypto" + kolideSessionIdHeaderKey = "X-Kolide-Session" + kolidePresenceDetectionInterval = "X-Kolide-Presence-Detection-Interval" ) type v2CmdRequestType struct { diff --git a/ee/localserver/server.go b/ee/localserver/server.go index 801b59e12..d34e08207 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -12,7 +12,6 @@ import ( "log/slog" "net" "net/http" - "strconv" "strings" "time" @@ -20,6 +19,7 @@ import ( "github.com/kolide/krypto/pkg/echelper" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/agent/types" + "github.com/kolide/launcher/ee/desktop/runner" "github.com/kolide/launcher/ee/presencedetection" "github.com/kolide/launcher/pkg/osquery" "github.com/kolide/launcher/pkg/traces" @@ -123,6 +123,8 @@ func New(ctx context.Context, k types.Knapsack) (*localServer, error) { // mux.Handle("/scheduledquery", ls.requestScheduledQueryHandler()) // curl localhost:40978/acceleratecontrol --data '{"interval":"250ms", "duration":"1s"}' // mux.Handle("/acceleratecontrol", ls.requestAccelerateControlHandler()) + // curl localhost:40978/id + // mux.Handle("/id", ls.requestIdHandler()) srv := &http.Server{ Handler: otelhttp.NewHandler( @@ -406,23 +408,23 @@ func (ls *localServer) rateLimitHandler(next http.Handler) http.Handler { func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // can test this by adding an unauthed endpoint to the mux and running, for example: - // curl -H "X-Kolide-Presence-Detection-Interval: 0" localhost:12519/id - detectionIntervalSecondsStr := r.Header.Get(kolidePresenceDetectionIntervalSecondsKey) + // curl -H "X-Kolide-Presence-Detection-Interval: 10s" localhost:12519/id + detectionIntervalStr := r.Header.Get(kolidePresenceDetectionInterval) // no presence detection requested - if detectionIntervalSecondsStr == "" { + if detectionIntervalStr == "" { next.ServeHTTP(w, r) } - detectionIntervalSeconds, err := strconv.Atoi(detectionIntervalSecondsStr) + detectionIntervalDuration, err := time.ParseDuration(detectionIntervalStr) if err != nil { - http.Error(w, err.Error(), http.StatusUnauthorized) + http.Error(w, err.Error(), http.StatusBadRequest) return } - success, err := ls.presenceDetector.DetectForConsoleUser("detect user presence", time.Duration(detectionIntervalSeconds)*time.Second) + // TODO: decide how we want to get the reason. Pull from header? + success, err := runner.InstanceDetectPresence("authenticate a thing", detectionIntervalDuration) if err != nil { http.Error(w, err.Error(), http.StatusUnauthorized) return diff --git a/ee/presencedetection/presencedetection.go b/ee/presencedetection/presencedetection.go index 1565d2e5e..e3e66b490 100644 --- a/ee/presencedetection/presencedetection.go +++ b/ee/presencedetection/presencedetection.go @@ -1,19 +1,9 @@ package presencedetection import ( - "context" - "encoding/base64" - "encoding/json" - "errors" "fmt" - "os" - "os/exec" - "os/user" - "strings" "sync" "time" - - "github.com/kolide/launcher/ee/consoleuser" ) type PresenceDetectionResponse struct { @@ -26,7 +16,7 @@ type PresenceDetector struct { mutext sync.Mutex } -func (pd *PresenceDetector) DetectForConsoleUser(reason string, detectionInterval time.Duration) (bool, error) { +func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time.Duration) (bool, error) { pd.mutext.Lock() defer pd.mutext.Unlock() @@ -35,82 +25,14 @@ func (pd *PresenceDetector) DetectForConsoleUser(reason string, detectionInterva return true, nil } - executablePath, err := os.Executable() - if err != nil { - return false, fmt.Errorf("could not get executable path: %w", err) - } - - consoleUserUids, err := consoleuser.CurrentUids(context.TODO()) + success, err := Detect(reason) if err != nil { - return false, fmt.Errorf("could not get console user: %w", err) + return false, fmt.Errorf("detecting presence: %w", err) } - if len(consoleUserUids) == 0 { - return false, errors.New("no console user found") - } - - runningUserUid := consoleUserUids[0] - - // Ensure that we handle a non-root current user appropriately - currentUser, err := user.Current() - if err != nil { - return false, fmt.Errorf("getting current user: %w", err) - } - - runningUser, err := user.LookupId(runningUserUid) - if err != nil || runningUser == nil { - return false, fmt.Errorf("looking up user with uid %s: %w", runningUserUid, err) - } - - cmd := exec.Command(executablePath, "detect-presence", reason) //nolint:forbidigo // We trust that the launcher executable path is correct, so we don't need to use allowedcmd - - // Update command so that we're prepending `launchctl asuser $UID sudo --preserve-env -u $runningUser` to the launcher desktop command. - // We need to run with `launchctl asuser` in order to get the user context, which is required to be able to send notifications. - // We need `sudo -u $runningUser` to set the UID on the command correctly -- necessary for, among other things, correctly observing - // light vs dark mode. - // We need --preserve-env for sudo in order to avoid clearing SOCKET_PATH, AUTHTOKEN, etc that are necessary for the desktop - // process to run. - cmd.Path = "/bin/launchctl" - updatedCmdArgs := append([]string{"/bin/launchctl", "asuser", runningUserUid, "sudo", "--preserve-env", "-u", runningUser.Username}, cmd.Args...) - cmd.Args = updatedCmdArgs - - if currentUser.Uid != "0" && currentUser.Uid != runningUserUid { - // if the user is running for another user, we have an error because we can't set credentials - return false, fmt.Errorf("current user %s is not root and does not match running user, can't start process for other user %s", currentUser.Uid, runningUserUid) - } - - out, err := cmd.CombinedOutput() - if err != nil { - return false, fmt.Errorf("could not run command: %w", err) - } - - outStr := string(out) - - // get last line of outstr - lastLine := "" - for _, line := range strings.Split(outStr, "\n") { - if line != "" { - lastLine = line - } - } - - outDecoded, err := base64.StdEncoding.DecodeString(lastLine) - if err != nil { - return false, fmt.Errorf("could not decode output: %w", err) - } - - response := PresenceDetectionResponse{} - if err := json.Unmarshal(outDecoded, &response); err != nil { - return false, fmt.Errorf("could not unmarshal response: %w", err) - } - - if response.Success { + if success { pd.lastDetectionUTC = time.Now().UTC() } - if response.Error != "" { - return response.Success, errors.New(response.Error) - } - - return response.Success, nil + return success, nil } From c1eb82dfd9865f9f641bf01e74a1a50189734312 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Fri, 20 Sep 2024 12:30:55 -0700 Subject: [PATCH 07/23] clean up, lint --- ee/desktop/runner/runner.go | 7 +++++-- ee/localserver/server.go | 3 --- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index 6473ef858..edc6efec1 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -548,10 +548,9 @@ func (r *DesktopUsersProcessesRunner) refreshMenu() { return } - // Tell any running desktop user processes that they should show menu and refresh the latest menu data + // Tell any running desktop user processes that they should refresh the latest menu data for uid, proc := range r.uidProcs { client := client.New(r.userServerAuthToken, proc.socketPath) - if err := client.Refresh(); err != nil { r.slogger.Log(context.TODO(), slog.LevelError, "sending refresh command to user desktop process", @@ -714,6 +713,10 @@ func (r *DesktopUsersProcessesRunner) spawnForUser(ctx context.Context, uid stri client := client.New(r.userServerAuthToken, socketPath) pingFunc := client.Ping + + // if the desktop is enabled, we want to show the desktop + // just perform this instead of ping to verify the desktop is running + // and show it right away if r.knapsack.DesktopEnabled() { pingFunc = client.ShowDesktop } diff --git a/ee/localserver/server.go b/ee/localserver/server.go index d34e08207..fc7efaa03 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -20,7 +20,6 @@ import ( "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/agent/types" "github.com/kolide/launcher/ee/desktop/runner" - "github.com/kolide/launcher/ee/presencedetection" "github.com/kolide/launcher/pkg/osquery" "github.com/kolide/launcher/pkg/traces" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" @@ -58,8 +57,6 @@ type localServer struct { serverKey *rsa.PublicKey serverEcKey *ecdsa.PublicKey - - presenceDetector presencedetection.PresenceDetector } const ( From 635ec1b80cd2fb459a82d5d3ab981a11c8624c35 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 23 Sep 2024 10:14:29 -0700 Subject: [PATCH 08/23] look for reason in header with default, remove unneeded struct --- ee/localserver/krypto-ec-middleware.go | 1 + ee/localserver/server.go | 14 ++++++++++---- ee/presencedetection/presencedetection.go | 5 ----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index 12bfea8ad..ca0f6245d 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -30,6 +30,7 @@ const ( kolideKryptoHeaderKey = "X-Kolide-Krypto" kolideSessionIdHeaderKey = "X-Kolide-Session" kolidePresenceDetectionInterval = "X-Kolide-Presence-Detection-Interval" + kolidePresenceDetectionReason = "X-Kolide-Presence-Detection-Reason" ) type v2CmdRequestType struct { diff --git a/ee/localserver/server.go b/ee/localserver/server.go index fc7efaa03..dbc6d9668 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -121,7 +121,7 @@ func New(ctx context.Context, k types.Knapsack) (*localServer, error) { // curl localhost:40978/acceleratecontrol --data '{"interval":"250ms", "duration":"1s"}' // mux.Handle("/acceleratecontrol", ls.requestAccelerateControlHandler()) // curl localhost:40978/id - // mux.Handle("/id", ls.requestIdHandler()) + mux.Handle("/id", ls.requestIdHandler()) srv := &http.Server{ Handler: otelhttp.NewHandler( @@ -406,7 +406,7 @@ func (ls *localServer) rateLimitHandler(next http.Handler) http.Handler { func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // can test this by adding an unauthed endpoint to the mux and running, for example: - // curl -H "X-Kolide-Presence-Detection-Interval: 10s" localhost:12519/id + // curl -H "X-Kolide-Presence-Detection-Interval: 10s" -H "X-Kolide-Presence-Detection-Reason: my reason" localhost:12519/id detectionIntervalStr := r.Header.Get(kolidePresenceDetectionInterval) // no presence detection requested @@ -420,8 +420,14 @@ func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler return } - // TODO: decide how we want to get the reason. Pull from header? - success, err := runner.InstanceDetectPresence("authenticate a thing", detectionIntervalDuration) + // set a default reason, on macos the popup will look like "Kolide is trying to authenticate." + reason := "authenticate" + reasonHeader := r.Header.Get(kolidePresenceDetectionReason) + if reasonHeader != "" { + reason = reasonHeader + } + + success, err := runner.InstanceDetectPresence(reason, detectionIntervalDuration) if err != nil { http.Error(w, err.Error(), http.StatusUnauthorized) return diff --git a/ee/presencedetection/presencedetection.go b/ee/presencedetection/presencedetection.go index e3e66b490..bc532bcbc 100644 --- a/ee/presencedetection/presencedetection.go +++ b/ee/presencedetection/presencedetection.go @@ -6,11 +6,6 @@ import ( "time" ) -type PresenceDetectionResponse struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` -} - type PresenceDetector struct { lastDetectionUTC time.Time mutext sync.Mutex From c87a1284584d98c098d2505e924f630c327d8dd6 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 23 Sep 2024 10:24:44 -0700 Subject: [PATCH 09/23] put c in own file --- ee/localserver/server.go | 2 +- ee/presencedetection/auth.h | 15 ++++ ee/presencedetection/auth.m | 65 +++++++++++++++++ .../presencedetection_darwin.go | 71 +------------------ 4 files changed, 82 insertions(+), 71 deletions(-) create mode 100644 ee/presencedetection/auth.h create mode 100644 ee/presencedetection/auth.m diff --git a/ee/localserver/server.go b/ee/localserver/server.go index dbc6d9668..b80320b4a 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -121,7 +121,7 @@ func New(ctx context.Context, k types.Knapsack) (*localServer, error) { // curl localhost:40978/acceleratecontrol --data '{"interval":"250ms", "duration":"1s"}' // mux.Handle("/acceleratecontrol", ls.requestAccelerateControlHandler()) // curl localhost:40978/id - mux.Handle("/id", ls.requestIdHandler()) + // mux.Handle("/id", ls.requestIdHandler()) srv := &http.Server{ Handler: otelhttp.NewHandler( diff --git a/ee/presencedetection/auth.h b/ee/presencedetection/auth.h new file mode 100644 index 000000000..31631ce5c --- /dev/null +++ b/ee/presencedetection/auth.h @@ -0,0 +1,15 @@ +// auth.h +#ifndef AUTH_H +#define AUTH_H + +#include + +struct AuthResult { + bool success; // true for success, false for failure + char* error_msg; // Error message if any + int error_code; // Error code if any +}; + +struct AuthResult Authenticate(char const* reason); + +#endif diff --git a/ee/presencedetection/auth.m b/ee/presencedetection/auth.m new file mode 100644 index 000000000..59b46e7e7 --- /dev/null +++ b/ee/presencedetection/auth.m @@ -0,0 +1,65 @@ +// auth.m +#import +#include "auth.h" + +struct AuthResult Authenticate(char const* reason) { + struct AuthResult authResult; + LAContext *myContext = [[LAContext alloc] init]; + NSError *authError = nil; + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + NSString *nsReason = [NSString stringWithUTF8String:reason]; + __block bool success = false; + __block NSString *errorMessage = nil; + __block int errorCode = 0; + + // Use LAPolicyDeviceOwnerAuthentication to allow biometrics and password fallback + if ([myContext canEvaluatePolicy:LAPolicyDeviceOwnerAuthentication error:&authError]) { + [myContext evaluatePolicy:LAPolicyDeviceOwnerAuthentication + localizedReason:nsReason + reply:^(BOOL policySuccess, NSError *error) { + if (policySuccess) { + success = true; // Authentication successful + } else { + success = false; + errorCode = (int)[error code]; + errorMessage = [error localizedDescription]; + if (error.code == LAErrorUserFallback || error.code == LAErrorAuthenticationFailed) { + // Prompting for password + [myContext evaluatePolicy:LAPolicyDeviceOwnerAuthentication + localizedReason:@"Please enter your password" + reply:^(BOOL pwdSuccess, NSError *error) { + if (pwdSuccess) { + success = true; + } else { + success = false; + errorCode = (int)[error code]; + errorMessage = [error localizedDescription]; + } + dispatch_semaphore_signal(sema); + }]; + } else { + errorCode = (int)[error code]; + errorMessage = [error localizedDescription]; + } + } + dispatch_semaphore_signal(sema); + }]; + } else { + success = false; // Cannot evaluate policy + errorCode = (int)[authError code]; + errorMessage = [authError localizedDescription]; + } + + dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER); + dispatch_release(sema); + + authResult.success = success; + authResult.error_code = errorCode; + if (errorMessage != nil) { + authResult.error_msg = strdup([errorMessage UTF8String]); // Copy error message to C string + } else { + authResult.error_msg = NULL; + } + + return authResult; +} diff --git a/ee/presencedetection/presencedetection_darwin.go b/ee/presencedetection/presencedetection_darwin.go index 8f7ebe79a..40f76d823 100644 --- a/ee/presencedetection/presencedetection_darwin.go +++ b/ee/presencedetection/presencedetection_darwin.go @@ -7,76 +7,7 @@ package presencedetection #cgo CFLAGS: -x objective-c -fmodules -fblocks #cgo LDFLAGS: -framework CoreFoundation -framework LocalAuthentication -framework Foundation #include -#include -#import - -struct AuthResult { - bool success; // true for success, false for failure - char* error_msg; // Error message if any - int error_code; // Error code if any -}; - -struct AuthResult Authenticate(char const* reason) { - struct AuthResult authResult; - LAContext *myContext = [[LAContext alloc] init]; - NSError *authError = nil; - dispatch_semaphore_t sema = dispatch_semaphore_create(0); - NSString *nsReason = [NSString stringWithUTF8String:reason]; - __block bool success = false; - __block NSString *errorMessage = nil; - __block int errorCode = 0; - - // Use LAPolicyDeviceOwnerAuthentication to allow biometrics and password fallback - if ([myContext canEvaluatePolicy:LAPolicyDeviceOwnerAuthentication error:&authError]) { - [myContext evaluatePolicy:LAPolicyDeviceOwnerAuthentication - localizedReason:nsReason - reply:^(BOOL policySuccess, NSError *error) { - if (policySuccess) { - success = true; // Authentication successful - } else { - success = false; - errorCode = (int)[error code]; - errorMessage = [error localizedDescription]; - if (error.code == LAErrorUserFallback || error.code == LAErrorAuthenticationFailed) { - // Prompting for password - [myContext evaluatePolicy:LAPolicyDeviceOwnerAuthentication - localizedReason:@"Please enter your password" - reply:^(BOOL pwdSuccess, NSError *error) { - if (pwdSuccess) { - success = true; - } else { - success = false; - errorCode = (int)[error code]; - errorMessage = [error localizedDescription]; - } - dispatch_semaphore_signal(sema); - }]; - } else { - errorCode = (int)[error code]; - errorMessage = [error localizedDescription]; - } - } - dispatch_semaphore_signal(sema); - }]; - } else { - success = false; // Cannot evaluate policy - errorCode = (int)[authError code]; - errorMessage = [authError localizedDescription]; - } - - dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER); - dispatch_release(sema); - - authResult.success = success; - authResult.error_code = errorCode; - if (errorMessage != nil) { - authResult.error_msg = strdup([errorMessage UTF8String]); // Copy error message to C string - } else { - authResult.error_msg = NULL; - } - - return authResult; -} +#include "auth.h" */ import "C" import ( From 47fbb3251b1451885911a108b9bcaab80efbbbb8 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 23 Sep 2024 10:38:26 -0700 Subject: [PATCH 10/23] dont use desktop runner singleton --- cmd/launcher/launcher.go | 1 + ee/desktop/runner/runner.go | 48 ++++++++++++++----------------- ee/localserver/request-id_test.go | 2 +- ee/localserver/server.go | 12 ++++++-- ee/localserver/server_test.go | 2 +- 5 files changed, 34 insertions(+), 31 deletions(-) diff --git a/cmd/launcher/launcher.go b/cmd/launcher/launcher.go index 9d39daa44..dea7592c3 100644 --- a/cmd/launcher/launcher.go +++ b/cmd/launcher/launcher.go @@ -504,6 +504,7 @@ func runLauncher(ctx context.Context, cancel func(), multiSlogger, systemMultiSl ls, err := localserver.New( ctx, k, + runner, ) if err != nil { diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index edc6efec1..a46f3843c 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -88,32 +88,6 @@ func InstanceDesktopProcessRecords() map[string]processRecord { return instance.uidProcs } -func InstanceDetectPresence(reason string, interval time.Duration) (bool, error) { - if instance == nil { - return false, errors.New("no instance of DesktopUsersProcessesRunner") - } - - if instance.uidProcs == nil || len(instance.uidProcs) == 0 { - return false, errors.New("no desktop processes running") - } - - var lastErr error - for _, proc := range instance.uidProcs { - client := client.New(instance.userServerAuthToken, proc.socketPath) - success, err := client.DetectPresence(reason, interval) - - // not sure how to handle the possiblity of multiple users - // so just return the first success - if success { - return success, err - } - - lastErr = err - } - - return false, fmt.Errorf("no desktop processes detected presence, last error: %w", lastErr) -} - // DesktopUsersProcessesRunner creates a launcher desktop process each time it detects // a new console (GUI) user. If the current console user's desktop process dies, it // will create a new one. @@ -308,6 +282,28 @@ func (r *DesktopUsersProcessesRunner) Interrupt(_ error) { ) } +func (r *DesktopUsersProcessesRunner) DetectPresence(reason string, interval time.Duration) (bool, error) { + if r.uidProcs == nil || len(r.uidProcs) == 0 { + return false, errors.New("no desktop processes running") + } + + var lastErr error + for _, proc := range r.uidProcs { + client := client.New(r.userServerAuthToken, proc.socketPath) + success, err := client.DetectPresence(reason, interval) + + // not sure how to handle the possiblity of multiple users + // so just return the first success + if success { + return success, err + } + + lastErr = err + } + + return false, fmt.Errorf("no desktop processes detected presence, last error: %w", lastErr) +} + // killDesktopProcesses kills any existing desktop processes func (r *DesktopUsersProcessesRunner) killDesktopProcesses(ctx context.Context) { wgDone := make(chan struct{}) diff --git a/ee/localserver/request-id_test.go b/ee/localserver/request-id_test.go index f917571ca..8873bb34c 100644 --- a/ee/localserver/request-id_test.go +++ b/ee/localserver/request-id_test.go @@ -61,7 +61,7 @@ func Test_localServer_requestIdHandler(t *testing.T) { func testServer(t *testing.T, k types.Knapsack) *localServer { require.NoError(t, osquery.SetupLauncherKeys(k.ConfigStore())) - server, err := New(context.TODO(), k) + server, err := New(context.TODO(), k, nil) require.NoError(t, err) return server } diff --git a/ee/localserver/server.go b/ee/localserver/server.go index b80320b4a..ea689fe5e 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -19,7 +19,6 @@ import ( "github.com/kolide/krypto/pkg/echelper" "github.com/kolide/launcher/ee/agent" "github.com/kolide/launcher/ee/agent/types" - "github.com/kolide/launcher/ee/desktop/runner" "github.com/kolide/launcher/pkg/osquery" "github.com/kolide/launcher/pkg/traces" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" @@ -57,6 +56,8 @@ type localServer struct { serverKey *rsa.PublicKey serverEcKey *ecdsa.PublicKey + + presenceDetector presenceDetector } const ( @@ -64,7 +65,11 @@ const ( defaultRateBurst = 10 ) -func New(ctx context.Context, k types.Knapsack) (*localServer, error) { +type presenceDetector interface { + DetectPresence(reason string, interval time.Duration) (bool, error) +} + +func New(ctx context.Context, k types.Knapsack, presenceDetector presenceDetector) (*localServer, error) { _, span := traces.StartSpan(ctx) defer span.End() @@ -75,6 +80,7 @@ func New(ctx context.Context, k types.Knapsack) (*localServer, error) { kolideServer: k.KolideServerURL(), myLocalDbSigner: agent.LocalDbKeys(), myLocalHardwareSigner: agent.HardwareKeys(), + presenceDetector: presenceDetector, } // TODO: As there may be things that adjust the keys during runtime, we need to persist that across @@ -427,7 +433,7 @@ func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler reason = reasonHeader } - success, err := runner.InstanceDetectPresence(reason, detectionIntervalDuration) + success, err := ls.presenceDetector.DetectPresence(reason, detectionIntervalDuration) if err != nil { http.Error(w, err.Error(), http.StatusUnauthorized) return diff --git a/ee/localserver/server_test.go b/ee/localserver/server_test.go index 83dfd5cdf..7e4732c06 100644 --- a/ee/localserver/server_test.go +++ b/ee/localserver/server_test.go @@ -36,7 +36,7 @@ func TestInterrupt_Multiple(t *testing.T) { recalculateInterval = 100 * time.Millisecond // Create the localserver - ls, err := New(context.TODO(), k) + ls, err := New(context.TODO(), k, nil) require.NoError(t, err) // Set the querier From 1fd46c9851969568b8816a667fe30efd0300566e Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 23 Sep 2024 10:49:45 -0700 Subject: [PATCH 11/23] build tags in c files --- ee/presencedetection/auth.h | 3 +++ ee/presencedetection/auth.m | 3 +++ 2 files changed, 6 insertions(+) diff --git a/ee/presencedetection/auth.h b/ee/presencedetection/auth.h index 31631ce5c..6d2538ced 100644 --- a/ee/presencedetection/auth.h +++ b/ee/presencedetection/auth.h @@ -1,3 +1,6 @@ +//go:build darwin +// +build darwin + // auth.h #ifndef AUTH_H #define AUTH_H diff --git a/ee/presencedetection/auth.m b/ee/presencedetection/auth.m index 59b46e7e7..6f3b1f52b 100644 --- a/ee/presencedetection/auth.m +++ b/ee/presencedetection/auth.m @@ -1,3 +1,6 @@ +//go:build darwin +// +build darwin + // auth.m #import #include "auth.h" From 888643fc65957bbea5d73842e70d1836687b4f9f Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 23 Sep 2024 11:21:13 -0700 Subject: [PATCH 12/23] presence detection tests --- ee/localserver/mocks/presenceDetector.go | 56 ++++++++++ .../presence-detection-middleware_test.go | 100 ++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 ee/localserver/mocks/presenceDetector.go create mode 100644 ee/localserver/presence-detection-middleware_test.go diff --git a/ee/localserver/mocks/presenceDetector.go b/ee/localserver/mocks/presenceDetector.go new file mode 100644 index 000000000..f3a08c477 --- /dev/null +++ b/ee/localserver/mocks/presenceDetector.go @@ -0,0 +1,56 @@ +// Code generated by mockery v2.44.1. DO NOT EDIT. + +package mocks + +import ( + time "time" + + mock "github.com/stretchr/testify/mock" +) + +// PresenceDetector is an autogenerated mock type for the presenceDetector type +type PresenceDetector struct { + mock.Mock +} + +// DetectPresence provides a mock function with given fields: reason, interval +func (_m *PresenceDetector) DetectPresence(reason string, interval time.Duration) (bool, error) { + ret := _m.Called(reason, interval) + + if len(ret) == 0 { + panic("no return value specified for DetectPresence") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(string, time.Duration) (bool, error)); ok { + return rf(reason, interval) + } + if rf, ok := ret.Get(0).(func(string, time.Duration) bool); ok { + r0 = rf(reason, interval) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(string, time.Duration) error); ok { + r1 = rf(reason, interval) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewPresenceDetector creates a new instance of PresenceDetector. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewPresenceDetector(t interface { + mock.TestingT + Cleanup(func()) +}) *PresenceDetector { + mock := &PresenceDetector{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/ee/localserver/presence-detection-middleware_test.go b/ee/localserver/presence-detection-middleware_test.go new file mode 100644 index 000000000..1d9de7c0c --- /dev/null +++ b/ee/localserver/presence-detection-middleware_test.go @@ -0,0 +1,100 @@ +package localserver + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/kolide/launcher/ee/localserver/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestPresenceDetectionHandler(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + expectDetectPresenceCall bool + intervalHeader, reasonHeader string + presenceDetectionSuccess bool + presenceDetectionError error + expectedStatusCode int + }{ + { + name: "no presence detection headers", + expectedStatusCode: http.StatusOK, + }, + { + name: "invalid presence detection interval", + intervalHeader: "invalid-interval", + expectedStatusCode: http.StatusBadRequest, + }, + { + name: "valid presence detection, detection fails", + expectDetectPresenceCall: true, + intervalHeader: "10s", + reasonHeader: "test reason", + presenceDetectionSuccess: false, + expectedStatusCode: http.StatusUnauthorized, + }, + { + name: "valid presence detection, detection succeeds", + expectDetectPresenceCall: true, + intervalHeader: "10s", + reasonHeader: "test reason", + presenceDetectionSuccess: true, + expectedStatusCode: http.StatusOK, + }, + { + name: "presence detection error", + expectDetectPresenceCall: true, + intervalHeader: "10s", + reasonHeader: "test reason", + presenceDetectionSuccess: false, + presenceDetectionError: assert.AnError, + expectedStatusCode: http.StatusUnauthorized, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + mockPresenceDetector := mocks.NewPresenceDetector(t) + + if tt.expectDetectPresenceCall { + mockPresenceDetector.On("DetectPresence", mock.AnythingOfType("string"), mock.AnythingOfType("Duration")).Return(tt.presenceDetectionSuccess, tt.presenceDetectionError) + } + + server := &localServer{ + presenceDetector: mockPresenceDetector, + } + + // Create a test handler for the middleware to call + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + // Wrap the test handler in the middleware + handlerToTest := server.presenceDetectionHandler(nextHandler) + + // Create a request with the specified headers + req := httptest.NewRequest("GET", "/", nil) + if tt.intervalHeader != "" { + req.Header.Add("X-Kolide-Presence-Detection-Interval", tt.intervalHeader) + } + + if tt.reasonHeader != "" { + req.Header.Add("X-Kolide-Presence-Detection-Reason", tt.reasonHeader) + } + + rr := httptest.NewRecorder() + handlerToTest.ServeHTTP(rr, req) + + require.Equal(t, tt.expectedStatusCode, rr.Code) + }) + } +} From ddc3adae3bb29755d717bc3a801f72f7d474a343 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Mon, 23 Sep 2024 14:04:09 -0700 Subject: [PATCH 13/23] test that headers are present after opening krypto challenge --- ee/localserver/krypto-ec-middleware_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ee/localserver/krypto-ec-middleware_test.go b/ee/localserver/krypto-ec-middleware_test.go index fa135795a..ba659da1c 100644 --- a/ee/localserver/krypto-ec-middleware_test.go +++ b/ee/localserver/krypto-ec-middleware_test.go @@ -36,6 +36,11 @@ func TestKryptoEcMiddleware(t *testing.T) { challengeData := []byte(ulid.New()) koldieSessionId := ulid.New() + cmdRequestHeaders := map[string][]string{ + "some_key": {ulid.New()}, + "other_key": {ulid.New()}, + } + cmdReqCallBackHeaders := map[string][]string{ kolideSessionIdHeaderKey: {koldieSessionId}, } @@ -44,6 +49,7 @@ func TestKryptoEcMiddleware(t *testing.T) { cmdReq := mustMarshal(t, v2CmdRequestType{ Path: "whatevs", Body: cmdReqBody, + Headers: cmdRequestHeaders, CallbackHeaders: cmdReqCallBackHeaders, }) @@ -143,6 +149,12 @@ func TestKryptoEcMiddleware(t *testing.T) { // this should match the responseData in the opened response if testHandler == nil { testHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + + // make sure all the request headers are present + for k, v := range cmdRequestHeaders { + require.Equal(t, v[0], r.Header.Get(k)) + } + reqBodyRaw, err := io.ReadAll(r.Body) require.NoError(t, err) defer r.Body.Close() From 9524d0da25c51d5fc9f8c4bc9ffa62f5218579f7 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Tue, 24 Sep 2024 09:36:22 -0700 Subject: [PATCH 14/23] update log --- ee/desktop/runner/runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index a46f3843c..bd45a510d 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -476,7 +476,7 @@ func (r *DesktopUsersProcessesRunner) FlagsChanged(flagKeys ...keys.FlagKey) { r.slogger.Log(context.TODO(), slog.LevelDebug, "desktop enabled set by control server", - "process_spawning_enabled", r.knapsack.DesktopEnabled(), + "desktop_enabled", r.knapsack.DesktopEnabled(), ) if !r.knapsack.DesktopEnabled() { From cd655421e9c7312fddeef927ae4a40b01865f5a8 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Tue, 24 Sep 2024 11:32:53 -0700 Subject: [PATCH 15/23] dont display desktop in test, just manage process --- ee/desktop/runner/runner_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ee/desktop/runner/runner_test.go b/ee/desktop/runner/runner_test.go index 211a9ace5..9e1cfcbf9 100644 --- a/ee/desktop/runner/runner_test.go +++ b/ee/desktop/runner/runner_test.go @@ -123,7 +123,10 @@ func TestDesktopUserProcessRunner_Execute(t *testing.T) { mockKnapsack.On("DesktopUpdateInterval").Return(time.Millisecond * 250) mockKnapsack.On("DesktopMenuRefreshInterval").Return(time.Millisecond * 250) mockKnapsack.On("KolideServerURL").Return("somewhere-over-the-rainbow.example.com") - mockKnapsack.On("DesktopEnabled").Return(true) + + // don't try to display desktop under test, causes CI flakeyness + // (we still test process management, just without trying to display the desktop) + mockKnapsack.On("DesktopEnabled").Return(false) mockKnapsack.On("Slogger").Return(slogger) mockKnapsack.On("InModernStandby").Return(false) mockKnapsack.On("SystrayRestartEnabled").Return(false).Maybe() From 6f3bb1a677f8b090a6c6d0dd80f729b136236f8d Mon Sep 17 00:00:00 2001 From: James Pickett Date: Tue, 24 Sep 2024 11:50:12 -0700 Subject: [PATCH 16/23] increase runner test start up time --- ee/desktop/runner/runner_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ee/desktop/runner/runner_test.go b/ee/desktop/runner/runner_test.go index 9e1cfcbf9..9b26444b1 100644 --- a/ee/desktop/runner/runner_test.go +++ b/ee/desktop/runner/runner_test.go @@ -123,10 +123,7 @@ func TestDesktopUserProcessRunner_Execute(t *testing.T) { mockKnapsack.On("DesktopUpdateInterval").Return(time.Millisecond * 250) mockKnapsack.On("DesktopMenuRefreshInterval").Return(time.Millisecond * 250) mockKnapsack.On("KolideServerURL").Return("somewhere-over-the-rainbow.example.com") - - // don't try to display desktop under test, causes CI flakeyness - // (we still test process management, just without trying to display the desktop) - mockKnapsack.On("DesktopEnabled").Return(false) + mockKnapsack.On("DesktopEnabled").Return(true) mockKnapsack.On("Slogger").Return(slogger) mockKnapsack.On("InModernStandby").Return(false) mockKnapsack.On("SystrayRestartEnabled").Return(false).Maybe() @@ -156,7 +153,7 @@ func TestDesktopUserProcessRunner_Execute(t *testing.T) { }() // let it run a few intervals - time.Sleep(r.updateInterval * 3) + time.Sleep(r.updateInterval * 6) r.Interrupt(nil) user, err := user.Current() From 15a981d5b10b32ef208a7a511330b5644a1a541d Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 26 Sep 2024 11:28:06 -0700 Subject: [PATCH 17/23] presence detector return durtion since last detection, add to local server response header --- ee/desktop/runner/runner.go | 18 ++-- ee/desktop/user/client/client.go | 14 ++- ee/desktop/user/server/server.go | 15 +++- ee/localserver/krypto-ec-middleware.go | 13 +-- ee/localserver/mocks/presenceDetector.go | 10 +-- .../presence-detection-middleware_test.go | 64 ++++++++------ ee/localserver/server.go | 35 +++++--- ee/presencedetection/presencedetection.go | 42 ++++++--- .../presencedetection_test.go | 86 +++++++++++++++++++ 9 files changed, 220 insertions(+), 77 deletions(-) create mode 100644 ee/presencedetection/presencedetection_test.go diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index bd45a510d..8a95be32d 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -29,6 +29,7 @@ import ( "github.com/kolide/launcher/ee/desktop/user/client" "github.com/kolide/launcher/ee/desktop/user/menu" "github.com/kolide/launcher/ee/desktop/user/notify" + "github.com/kolide/launcher/ee/presencedetection" "github.com/kolide/launcher/ee/ui/assets" "github.com/kolide/launcher/pkg/backoff" "github.com/kolide/launcher/pkg/rungroup" @@ -282,26 +283,25 @@ func (r *DesktopUsersProcessesRunner) Interrupt(_ error) { ) } -func (r *DesktopUsersProcessesRunner) DetectPresence(reason string, interval time.Duration) (bool, error) { +func (r *DesktopUsersProcessesRunner) DetectPresence(reason string, interval time.Duration) (time.Duration, error) { if r.uidProcs == nil || len(r.uidProcs) == 0 { - return false, errors.New("no desktop processes running") + return presencedetection.DetectionFailedDurationValue, errors.New("no desktop processes running") } var lastErr error for _, proc := range r.uidProcs { client := client.New(r.userServerAuthToken, proc.socketPath) - success, err := client.DetectPresence(reason, interval) + durationSinceLastDetection, err := client.DetectPresence(reason, interval) - // not sure how to handle the possiblity of multiple users - // so just return the first success - if success { - return success, err + if err != nil { + lastErr = err + continue } - lastErr = err + return durationSinceLastDetection, nil } - return false, fmt.Errorf("no desktop processes detected presence, last error: %w", lastErr) + return presencedetection.DetectionFailedDurationValue, fmt.Errorf("no desktop processes detected presence, last error: %w", lastErr) } // killDesktopProcesses kills any existing desktop processes diff --git a/ee/desktop/user/client/client.go b/ee/desktop/user/client/client.go index 6c82358ae..be7f83d5d 100644 --- a/ee/desktop/user/client/client.go +++ b/ee/desktop/user/client/client.go @@ -12,6 +12,7 @@ import ( "github.com/kolide/launcher/ee/desktop/user/notify" "github.com/kolide/launcher/ee/desktop/user/server" + "github.com/kolide/launcher/ee/presencedetection" ) type transport struct { @@ -62,13 +63,13 @@ func (c *client) ShowDesktop() error { return c.get("show") } -func (c *client) DetectPresence(reason string, interval time.Duration) (bool, error) { +func (c *client) DetectPresence(reason string, interval time.Duration) (time.Duration, error) { encodedReason := url.QueryEscape(reason) encodedInterval := url.QueryEscape(interval.String()) resp, requestErr := c.base.Get(fmt.Sprintf("http://unix/detect_presence?reason=%s&interval=%s", encodedReason, encodedInterval)) if requestErr != nil { - return false, fmt.Errorf("getting presence: %w", requestErr) + return presencedetection.DetectionFailedDurationValue, fmt.Errorf("getting presence: %w", requestErr) } var response server.DetectPresenceResponse @@ -76,7 +77,7 @@ func (c *client) DetectPresence(reason string, interval time.Duration) (bool, er defer resp.Body.Close() if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { - return false, fmt.Errorf("decoding response: %w", err) + return presencedetection.DetectionFailedDurationValue, fmt.Errorf("decoding response: %w", err) } } @@ -85,7 +86,12 @@ func (c *client) DetectPresence(reason string, interval time.Duration) (bool, er err = errors.New(response.Error) } - return response.Success, err + durationSinceLastDetection, parseErr := time.ParseDuration(response.DurationSinceLastDetection) + if parseErr != nil { + return presencedetection.DetectionFailedDurationValue, fmt.Errorf("parsing time since last detection: %w", parseErr) + } + + return durationSinceLastDetection, err } func (c *client) Notify(n notify.Notification) error { diff --git a/ee/desktop/user/server/server.go b/ee/desktop/user/server/server.go index 8efa2c6e6..fd674a4ce 100644 --- a/ee/desktop/user/server/server.go +++ b/ee/desktop/user/server/server.go @@ -174,8 +174,8 @@ func (s *UserServer) showDesktop(w http.ResponseWriter, req *http.Request) { } type DetectPresenceResponse struct { - Success bool `json:"success"` - Error string `json:"error,omitempty"` + DurationSinceLastDetection string `json:"duration_since_last_detection,omitempty"` + Error string `json:"error,omitempty"` } func (s *UserServer) detectPresence(w http.ResponseWriter, req *http.Request) { @@ -201,13 +201,20 @@ func (s *UserServer) detectPresence(w http.ResponseWriter, req *http.Request) { } // detect presence - success, err := s.presenceDetector.DetectPresence(reason, interval) + durationSinceLastDetection, err := s.presenceDetector.DetectPresence(reason, interval) response := DetectPresenceResponse{ - Success: success, + DurationSinceLastDetection: durationSinceLastDetection.String(), } if err != nil { response.Error = err.Error() + + s.slogger.Log(context.TODO(), slog.LevelDebug, + "detecting presence", + "reason", reason, + "interval", interval, + "err", err, + ) } // convert response to json diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index ca0f6245d..028d28bf3 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -25,12 +25,13 @@ import ( ) const ( - timestampValidityRange = 150 - kolideKryptoEccHeader20230130Value = "2023-01-30" - kolideKryptoHeaderKey = "X-Kolide-Krypto" - kolideSessionIdHeaderKey = "X-Kolide-Session" - kolidePresenceDetectionInterval = "X-Kolide-Presence-Detection-Interval" - kolidePresenceDetectionReason = "X-Kolide-Presence-Detection-Reason" + timestampValidityRange = 150 + kolideKryptoEccHeader20230130Value = "2023-01-30" + kolideKryptoHeaderKey = "X-Kolide-Krypto" + kolideSessionIdHeaderKey = "X-Kolide-Session" + kolidePresenceDetectionInterval = "X-Kolide-Presence-Detection-Interval" + kolidePresenceDetectionReason = "X-Kolide-Presence-Detection-Reason" + kolideDurationSinceLastPresenceDetection = "X-Kolide-Duration-Since-Last-Presence-Detection" ) type v2CmdRequestType struct { diff --git a/ee/localserver/mocks/presenceDetector.go b/ee/localserver/mocks/presenceDetector.go index f3a08c477..0af0518f7 100644 --- a/ee/localserver/mocks/presenceDetector.go +++ b/ee/localserver/mocks/presenceDetector.go @@ -14,22 +14,22 @@ type PresenceDetector struct { } // DetectPresence provides a mock function with given fields: reason, interval -func (_m *PresenceDetector) DetectPresence(reason string, interval time.Duration) (bool, error) { +func (_m *PresenceDetector) DetectPresence(reason string, interval time.Duration) (time.Duration, error) { ret := _m.Called(reason, interval) if len(ret) == 0 { panic("no return value specified for DetectPresence") } - var r0 bool + var r0 time.Duration var r1 error - if rf, ok := ret.Get(0).(func(string, time.Duration) (bool, error)); ok { + if rf, ok := ret.Get(0).(func(string, time.Duration) (time.Duration, error)); ok { return rf(reason, interval) } - if rf, ok := ret.Get(0).(func(string, time.Duration) bool); ok { + if rf, ok := ret.Get(0).(func(string, time.Duration) time.Duration); ok { r0 = rf(reason, interval) } else { - r0 = ret.Get(0).(bool) + r0 = ret.Get(0).(time.Duration) } if rf, ok := ret.Get(1).(func(string, time.Duration) error); ok { diff --git a/ee/localserver/presence-detection-middleware_test.go b/ee/localserver/presence-detection-middleware_test.go index 1d9de7c0c..d00e8d3f6 100644 --- a/ee/localserver/presence-detection-middleware_test.go +++ b/ee/localserver/presence-detection-middleware_test.go @@ -4,8 +4,11 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/kolide/launcher/ee/localserver/mocks" + "github.com/kolide/launcher/ee/presencedetection" + "github.com/kolide/launcher/pkg/log/multislogger" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -15,16 +18,18 @@ func TestPresenceDetectionHandler(t *testing.T) { t.Parallel() tests := []struct { - name string - expectDetectPresenceCall bool - intervalHeader, reasonHeader string - presenceDetectionSuccess bool - presenceDetectionError error - expectedStatusCode int + name string + expectDetectPresenceCall bool + intervalHeader, reasonHeader string + durationSinceLastDetection time.Duration + presenceDetectionError error + shouldHavePresenceDetectionDurationResponseHeader bool + expectedStatusCode int }{ { name: "no presence detection headers", expectedStatusCode: http.StatusOK, + shouldHavePresenceDetectionDurationResponseHeader: false, }, { name: "invalid presence detection interval", @@ -32,29 +37,32 @@ func TestPresenceDetectionHandler(t *testing.T) { expectedStatusCode: http.StatusBadRequest, }, { - name: "valid presence detection, detection fails", - expectDetectPresenceCall: true, - intervalHeader: "10s", - reasonHeader: "test reason", - presenceDetectionSuccess: false, - expectedStatusCode: http.StatusUnauthorized, + name: "valid presence detection, detection fails", + expectDetectPresenceCall: true, + intervalHeader: "10s", + reasonHeader: "test reason", + durationSinceLastDetection: presencedetection.DetectionFailedDurationValue, + expectedStatusCode: http.StatusOK, + shouldHavePresenceDetectionDurationResponseHeader: true, }, { - name: "valid presence detection, detection succeeds", - expectDetectPresenceCall: true, - intervalHeader: "10s", - reasonHeader: "test reason", - presenceDetectionSuccess: true, - expectedStatusCode: http.StatusOK, + name: "valid presence detection, detection succeeds", + expectDetectPresenceCall: true, + intervalHeader: "10s", + reasonHeader: "test reason", + durationSinceLastDetection: 0, + expectedStatusCode: http.StatusOK, + shouldHavePresenceDetectionDurationResponseHeader: true, }, { - name: "presence detection error", - expectDetectPresenceCall: true, - intervalHeader: "10s", - reasonHeader: "test reason", - presenceDetectionSuccess: false, - presenceDetectionError: assert.AnError, - expectedStatusCode: http.StatusUnauthorized, + name: "presence detection error", + expectDetectPresenceCall: true, + intervalHeader: "10s", + reasonHeader: "test reason", + durationSinceLastDetection: presencedetection.DetectionFailedDurationValue, + presenceDetectionError: assert.AnError, + expectedStatusCode: http.StatusOK, + shouldHavePresenceDetectionDurationResponseHeader: true, }, } @@ -66,11 +74,12 @@ func TestPresenceDetectionHandler(t *testing.T) { mockPresenceDetector := mocks.NewPresenceDetector(t) if tt.expectDetectPresenceCall { - mockPresenceDetector.On("DetectPresence", mock.AnythingOfType("string"), mock.AnythingOfType("Duration")).Return(tt.presenceDetectionSuccess, tt.presenceDetectionError) + mockPresenceDetector.On("DetectPresence", mock.AnythingOfType("string"), mock.AnythingOfType("Duration")).Return(tt.durationSinceLastDetection, tt.presenceDetectionError) } server := &localServer{ presenceDetector: mockPresenceDetector, + slogger: multislogger.NewNopLogger(), } // Create a test handler for the middleware to call @@ -94,6 +103,9 @@ func TestPresenceDetectionHandler(t *testing.T) { rr := httptest.NewRecorder() handlerToTest.ServeHTTP(rr, req) + if tt.shouldHavePresenceDetectionDurationResponseHeader { + require.NotEmpty(t, rr.Header().Get(kolideDurationSinceLastPresenceDetection)) + } require.Equal(t, tt.expectedStatusCode, rr.Code) }) } diff --git a/ee/localserver/server.go b/ee/localserver/server.go index ea689fe5e..812268eaf 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -12,6 +12,7 @@ import ( "log/slog" "net" "net/http" + "runtime" "strings" "time" @@ -66,7 +67,7 @@ const ( ) type presenceDetector interface { - DetectPresence(reason string, interval time.Duration) (bool, error) + DetectPresence(reason string, interval time.Duration) (time.Duration, error) } func New(ctx context.Context, k types.Knapsack, presenceDetector presenceDetector) (*localServer, error) { @@ -127,7 +128,7 @@ func New(ctx context.Context, k types.Knapsack, presenceDetector presenceDetecto // curl localhost:40978/acceleratecontrol --data '{"interval":"250ms", "duration":"1s"}' // mux.Handle("/acceleratecontrol", ls.requestAccelerateControlHandler()) // curl localhost:40978/id - // mux.Handle("/id", ls.requestIdHandler()) + mux.Handle("/id", ls.requestIdHandler()) srv := &http.Server{ Handler: otelhttp.NewHandler( @@ -411,6 +412,12 @@ func (ls *localServer) rateLimitHandler(next http.Handler) http.Handler { func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + + if runtime.GOOS != "darwin" { + next.ServeHTTP(w, r) + return + } + // can test this by adding an unauthed endpoint to the mux and running, for example: // curl -H "X-Kolide-Presence-Detection-Interval: 10s" -H "X-Kolide-Presence-Detection-Reason: my reason" localhost:12519/id detectionIntervalStr := r.Header.Get(kolidePresenceDetectionInterval) @@ -422,6 +429,8 @@ func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler detectionIntervalDuration, err := time.ParseDuration(detectionIntervalStr) if err != nil { + // this is the only time this should returna non-200 status code + // asked for presence detection, but the interval is invalid http.Error(w, err.Error(), http.StatusBadRequest) return } @@ -433,17 +442,21 @@ func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler reason = reasonHeader } - success, err := ls.presenceDetector.DetectPresence(reason, detectionIntervalDuration) - if err != nil { - http.Error(w, err.Error(), http.StatusUnauthorized) - return - } + durationSinceLastDetection, err := ls.presenceDetector.DetectPresence(reason, detectionIntervalDuration) - if !success { - http.Error(w, "presence detection failed", http.StatusUnauthorized) - return - } + ls.slogger.Log(r.Context(), slog.LevelError, + "presence_detection", + "reason", reason, + "interval", detectionIntervalDuration, + "duration_since_last_detection", durationSinceLastDetection, + "err", err, + ) + + // if there was an error, we still want to return a 200 status code + // and send the request through + // allow the server to decide what to do based on last detection duration + w.Header().Add(kolideDurationSinceLastPresenceDetection, durationSinceLastDetection.String()) next.ServeHTTP(w, r) }) } diff --git a/ee/presencedetection/presencedetection.go b/ee/presencedetection/presencedetection.go index bc532bcbc..0cb2ab57f 100644 --- a/ee/presencedetection/presencedetection.go +++ b/ee/presencedetection/presencedetection.go @@ -6,28 +6,46 @@ import ( "time" ) +const DetectionFailedDurationValue = -1 * time.Second + type PresenceDetector struct { - lastDetectionUTC time.Time - mutext sync.Mutex + lastDetectionUTC time.Time + mutext sync.Mutex + hadOneSuccessfulDetection bool + // detectFunc that can be set for testing + detectFunc func(string) (bool, error) } -func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time.Duration) (bool, error) { +// DetectPresence checks if the user is present by detecting the presence of a user. +// It returns the duration since the last detection. +func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time.Duration) (time.Duration, error) { pd.mutext.Lock() defer pd.mutext.Unlock() - // Check if the last detection was within the detection interval - if time.Since(pd.lastDetectionUTC) < detectionInterval { - return true, nil + if pd.detectFunc == nil { + pd.detectFunc = Detect } - success, err := Detect(reason) - if err != nil { - return false, fmt.Errorf("detecting presence: %w", err) + // Check if the last detection was within the detection interval + if pd.hadOneSuccessfulDetection && time.Since(pd.lastDetectionUTC) < detectionInterval { + return time.Since(pd.lastDetectionUTC), nil } - if success { + success, err := pd.detectFunc(reason) + + switch { + case err != nil && pd.hadOneSuccessfulDetection: + return time.Since(pd.lastDetectionUTC), fmt.Errorf("detecting presence: %w", err) + + case err != nil: // error without initial successful detection + return DetectionFailedDurationValue, fmt.Errorf("detecting presence: %w", err) + + case success: pd.lastDetectionUTC = time.Now().UTC() - } + pd.hadOneSuccessfulDetection = true + return 0, nil - return success, nil + default: // failed detection without error, maybe not possible? + return time.Since(pd.lastDetectionUTC), fmt.Errorf("detection failed without error") + } } diff --git a/ee/presencedetection/presencedetection_test.go b/ee/presencedetection/presencedetection_test.go new file mode 100644 index 000000000..88c7e583a --- /dev/null +++ b/ee/presencedetection/presencedetection_test.go @@ -0,0 +1,86 @@ +package presencedetection + +import ( + "errors" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestPresenceDetector_DetectPresence(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + interval time.Duration + detectFunc func(string) (bool, error) + initialLastDetectionUTC time.Time + hadOneSuccessfulDetection bool + errAssertion assert.ErrorAssertionFunc + expectedLastDetectionDelta time.Duration + }{ + { + name: "first detection success", + detectFunc: func(string) (bool, error) { + return true, nil + }, + errAssertion: assert.NoError, + expectedLastDetectionDelta: 0, + }, + { + name: "detection within interval", + detectFunc: func(string) (bool, error) { + return false, errors.New("should not have called detectFunc, since within interval") + }, + errAssertion: assert.NoError, + initialLastDetectionUTC: time.Now().UTC(), + interval: time.Minute, + hadOneSuccessfulDetection: true, + }, + { + name: "error first detection", + detectFunc: func(string) (bool, error) { + return false, errors.New("error") + }, + errAssertion: assert.Error, + expectedLastDetectionDelta: -1, + }, + { + name: "error after first detection", + detectFunc: func(string) (bool, error) { + return false, errors.New("error") + }, + errAssertion: assert.Error, + initialLastDetectionUTC: time.Now().UTC(), + hadOneSuccessfulDetection: true, + }, + { + name: "detection failed without OS error", + detectFunc: func(string) (bool, error) { + return false, nil + }, + errAssertion: assert.Error, + initialLastDetectionUTC: time.Now().UTC(), + hadOneSuccessfulDetection: true, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + pd := &PresenceDetector{ + detectFunc: tt.detectFunc, + lastDetectionUTC: tt.initialLastDetectionUTC, + hadOneSuccessfulDetection: tt.hadOneSuccessfulDetection, + } + + timeSinceLastDetection, err := pd.DetectPresence("this is a test", tt.interval) + tt.errAssertion(t, err) + + delta := timeSinceLastDetection - tt.expectedLastDetectionDelta + assert.LessOrEqual(t, delta, time.Second) + }) + } +} From aa824b147d9f9f49568f1b698e13333a306254e3 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 26 Sep 2024 11:35:43 -0700 Subject: [PATCH 18/23] tweaks --- ee/desktop/runner/runner.go | 8 +++++--- ee/desktop/user/client/client.go | 6 +++--- ee/localserver/server.go | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index 8a95be32d..5c560f995 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -289,19 +289,21 @@ func (r *DesktopUsersProcessesRunner) DetectPresence(reason string, interval tim } var lastErr error + var lastDurationSinceLastDetection time.Duration + for _, proc := range r.uidProcs { client := client.New(r.userServerAuthToken, proc.socketPath) - durationSinceLastDetection, err := client.DetectPresence(reason, interval) + lastDurationSinceLastDetection, err := client.DetectPresence(reason, interval) if err != nil { lastErr = err continue } - return durationSinceLastDetection, nil + return lastDurationSinceLastDetection, nil } - return presencedetection.DetectionFailedDurationValue, fmt.Errorf("no desktop processes detected presence, last error: %w", lastErr) + return lastDurationSinceLastDetection, fmt.Errorf("no desktop processes detected presence, last error: %w", lastErr) } // killDesktopProcesses kills any existing desktop processes diff --git a/ee/desktop/user/client/client.go b/ee/desktop/user/client/client.go index be7f83d5d..935ce9e71 100644 --- a/ee/desktop/user/client/client.go +++ b/ee/desktop/user/client/client.go @@ -81,9 +81,9 @@ func (c *client) DetectPresence(reason string, interval time.Duration) (time.Dur } } - var err error + var detectionErr error if response.Error != "" { - err = errors.New(response.Error) + detectionErr = errors.New(response.Error) } durationSinceLastDetection, parseErr := time.ParseDuration(response.DurationSinceLastDetection) @@ -91,7 +91,7 @@ func (c *client) DetectPresence(reason string, interval time.Duration) (time.Dur return presencedetection.DetectionFailedDurationValue, fmt.Errorf("parsing time since last detection: %w", parseErr) } - return durationSinceLastDetection, err + return durationSinceLastDetection, detectionErr } func (c *client) Notify(n notify.Notification) error { diff --git a/ee/localserver/server.go b/ee/localserver/server.go index 812268eaf..5631ecaba 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -128,7 +128,7 @@ func New(ctx context.Context, k types.Knapsack, presenceDetector presenceDetecto // curl localhost:40978/acceleratecontrol --data '{"interval":"250ms", "duration":"1s"}' // mux.Handle("/acceleratecontrol", ls.requestAccelerateControlHandler()) // curl localhost:40978/id - mux.Handle("/id", ls.requestIdHandler()) + // mux.Handle("/id", ls.requestIdHandler()) srv := &http.Server{ Handler: otelhttp.NewHandler( @@ -419,7 +419,7 @@ func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler } // can test this by adding an unauthed endpoint to the mux and running, for example: - // curl -H "X-Kolide-Presence-Detection-Interval: 10s" -H "X-Kolide-Presence-Detection-Reason: my reason" localhost:12519/id + // curl -i -H "X-Kolide-Presence-Detection-Interval: 10s" -H "X-Kolide-Presence-Detection-Reason: my reason" localhost:12519/id detectionIntervalStr := r.Header.Get(kolidePresenceDetectionInterval) // no presence detection requested From 8747dd0c7571a5b1317fc05ef6dfb8b51b37aa47 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 26 Sep 2024 11:42:11 -0700 Subject: [PATCH 19/23] skip presence test on non darwin --- ee/localserver/presence-detection-middleware_test.go | 5 +++++ ee/localserver/server.go | 1 + 2 files changed, 6 insertions(+) diff --git a/ee/localserver/presence-detection-middleware_test.go b/ee/localserver/presence-detection-middleware_test.go index d00e8d3f6..2ff95031c 100644 --- a/ee/localserver/presence-detection-middleware_test.go +++ b/ee/localserver/presence-detection-middleware_test.go @@ -3,6 +3,7 @@ package localserver import ( "net/http" "net/http/httptest" + "runtime" "testing" "time" @@ -17,6 +18,10 @@ import ( func TestPresenceDetectionHandler(t *testing.T) { t.Parallel() + if runtime.GOOS != "darwin" { + t.Skip("test only runs on darwin until implemented for other OSes") + } + tests := []struct { name string expectDetectPresenceCall bool diff --git a/ee/localserver/server.go b/ee/localserver/server.go index 5631ecaba..e2a9620dc 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -413,6 +413,7 @@ func (ls *localServer) rateLimitHandler(next http.Handler) http.Handler { func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // presence detection is only supported on macos currently if runtime.GOOS != "darwin" { next.ServeHTTP(w, r) return From e12a3326147950f488e74e1f93d303b382bec306 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Thu, 26 Sep 2024 14:51:54 -0700 Subject: [PATCH 20/23] simplify presence detector --- ee/presencedetection/mocks/detectorIface.go | 52 +++++++++ ee/presencedetection/presencedetection.go | 33 ++++-- .../presencedetection_test.go | 105 +++++++++++------- 3 files changed, 137 insertions(+), 53 deletions(-) create mode 100644 ee/presencedetection/mocks/detectorIface.go diff --git a/ee/presencedetection/mocks/detectorIface.go b/ee/presencedetection/mocks/detectorIface.go new file mode 100644 index 000000000..19bd705d4 --- /dev/null +++ b/ee/presencedetection/mocks/detectorIface.go @@ -0,0 +1,52 @@ +// Code generated by mockery v2.44.1. DO NOT EDIT. + +package mocks + +import mock "github.com/stretchr/testify/mock" + +// DetectorIface is an autogenerated mock type for the detectorIface type +type DetectorIface struct { + mock.Mock +} + +// Detect provides a mock function with given fields: reason +func (_m *DetectorIface) Detect(reason string) (bool, error) { + ret := _m.Called(reason) + + if len(ret) == 0 { + panic("no return value specified for Detect") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(string) (bool, error)); ok { + return rf(reason) + } + if rf, ok := ret.Get(0).(func(string) bool); ok { + r0 = rf(reason) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(reason) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewDetectorIface creates a new instance of DetectorIface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewDetectorIface(t interface { + mock.TestingT + Cleanup(func()) +}) *DetectorIface { + mock := &DetectorIface{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/ee/presencedetection/presencedetection.go b/ee/presencedetection/presencedetection.go index 0cb2ab57f..95f3abe55 100644 --- a/ee/presencedetection/presencedetection.go +++ b/ee/presencedetection/presencedetection.go @@ -9,11 +9,21 @@ import ( const DetectionFailedDurationValue = -1 * time.Second type PresenceDetector struct { - lastDetectionUTC time.Time - mutext sync.Mutex - hadOneSuccessfulDetection bool - // detectFunc that can be set for testing - detectFunc func(string) (bool, error) + lastDetectionUTC time.Time + mutext sync.Mutex + // detector is an interface to allow for mocking in tests + detector detectorIface +} + +// just exists for testing purposes +type detectorIface interface { + Detect(reason string) (bool, error) +} + +type detector struct{} + +func (d *detector) Detect(reason string) (bool, error) { + return Detect(reason) } // DetectPresence checks if the user is present by detecting the presence of a user. @@ -22,19 +32,21 @@ func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time pd.mutext.Lock() defer pd.mutext.Unlock() - if pd.detectFunc == nil { - pd.detectFunc = Detect + if pd.detector == nil { + pd.detector = &detector{} } + hadHadSuccessfulDetection := pd.lastDetectionUTC != time.Time{} + // Check if the last detection was within the detection interval - if pd.hadOneSuccessfulDetection && time.Since(pd.lastDetectionUTC) < detectionInterval { + if hadHadSuccessfulDetection && time.Since(pd.lastDetectionUTC) < detectionInterval { return time.Since(pd.lastDetectionUTC), nil } - success, err := pd.detectFunc(reason) + success, err := pd.detector.Detect(reason) switch { - case err != nil && pd.hadOneSuccessfulDetection: + case err != nil && hadHadSuccessfulDetection: return time.Since(pd.lastDetectionUTC), fmt.Errorf("detecting presence: %w", err) case err != nil: // error without initial successful detection @@ -42,7 +54,6 @@ func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time case success: pd.lastDetectionUTC = time.Now().UTC() - pd.hadOneSuccessfulDetection = true return 0, nil default: // failed detection without error, maybe not possible? diff --git a/ee/presencedetection/presencedetection_test.go b/ee/presencedetection/presencedetection_test.go index 88c7e583a..6ed82f2fb 100644 --- a/ee/presencedetection/presencedetection_test.go +++ b/ee/presencedetection/presencedetection_test.go @@ -2,85 +2,106 @@ package presencedetection import ( "errors" + "math" "testing" "time" + "github.com/kolide/launcher/ee/presencedetection/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func TestPresenceDetector_DetectPresence(t *testing.T) { t.Parallel() tests := []struct { - name string - interval time.Duration - detectFunc func(string) (bool, error) - initialLastDetectionUTC time.Time - hadOneSuccessfulDetection bool - errAssertion assert.ErrorAssertionFunc - expectedLastDetectionDelta time.Duration + name string + interval time.Duration + detector func(t *testing.T) detectorIface + initialLastDetectionUTC time.Time + expectError bool }{ { - name: "first detection success", - detectFunc: func(string) (bool, error) { - return true, nil + name: "first detection success", + interval: 0, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(true, nil) + return d }, - errAssertion: assert.NoError, - expectedLastDetectionDelta: 0, }, { - name: "detection within interval", - detectFunc: func(string) (bool, error) { - return false, errors.New("should not have called detectFunc, since within interval") + name: "detection outside interval", + interval: time.Minute, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(true, nil) + return d }, - errAssertion: assert.NoError, - initialLastDetectionUTC: time.Now().UTC(), - interval: time.Minute, - hadOneSuccessfulDetection: true, + initialLastDetectionUTC: time.Now().UTC().Add(-time.Minute), }, { - name: "error first detection", - detectFunc: func(string) (bool, error) { - return false, errors.New("error") + name: "detection within interval", + interval: time.Minute, + detector: func(t *testing.T) detectorIface { + // should not be called, will get error if it is + return mocks.NewDetectorIface(t) }, - errAssertion: assert.Error, - expectedLastDetectionDelta: -1, + initialLastDetectionUTC: time.Now().UTC(), }, { - name: "error after first detection", - detectFunc: func(string) (bool, error) { - return false, errors.New("error") + name: "error first detection", + interval: 0, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(true, errors.New("error")) + return d }, - errAssertion: assert.Error, - initialLastDetectionUTC: time.Now().UTC(), - hadOneSuccessfulDetection: true, + expectError: true, }, { - name: "detection failed without OS error", - detectFunc: func(string) (bool, error) { - return false, nil + name: "error after first detection", + interval: 0, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(true, errors.New("error")) + return d }, - errAssertion: assert.Error, - initialLastDetectionUTC: time.Now().UTC(), - hadOneSuccessfulDetection: true, + initialLastDetectionUTC: time.Now().UTC(), + expectError: true, + }, + { + name: "detection failed without OS error", + interval: 0, + detector: func(t *testing.T) detectorIface { + d := mocks.NewDetectorIface(t) + d.On("Detect", mock.AnythingOfType("string")).Return(false, nil) + return d + }, + initialLastDetectionUTC: time.Now().UTC(), + expectError: true, }, } + for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() pd := &PresenceDetector{ - detectFunc: tt.detectFunc, - lastDetectionUTC: tt.initialLastDetectionUTC, - hadOneSuccessfulDetection: tt.hadOneSuccessfulDetection, + detector: tt.detector(t), + lastDetectionUTC: tt.initialLastDetectionUTC, } timeSinceLastDetection, err := pd.DetectPresence("this is a test", tt.interval) - tt.errAssertion(t, err) - delta := timeSinceLastDetection - tt.expectedLastDetectionDelta - assert.LessOrEqual(t, delta, time.Second) + if tt.expectError { + assert.Error(t, err) + return + } + + absDelta := math.Abs(timeSinceLastDetection.Seconds() - tt.interval.Seconds()) + assert.LessOrEqual(t, absDelta, tt.interval.Seconds()) }) } } From dcaa01e0946e9f9d2d322d06dc124e7ce08a4527 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Fri, 27 Sep 2024 09:14:58 -0700 Subject: [PATCH 21/23] presence detection via ec middleware --- ee/localserver/krypto-ec-middleware.go | 8 ++++++++ ee/localserver/krypto-ec-middleware_test.go | 16 +++++++++++++--- ee/localserver/server.go | 7 ++++--- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index 028d28bf3..37d17f357 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -344,6 +344,14 @@ func (e *kryptoEcMiddleware) Wrap(next http.Handler) http.Handler { w.Header().Add(kolideKryptoHeaderKey, kolideKryptoEccHeader20230130Value) + for k, v := range bhr.Header() { + if len(v) == 0 { + continue + } + + w.Header().Add(k, v[0]) + } + // arguable the png things here should be their own handler. But doing that means another layer // buffering the http response, so it feels a bit silly. When we ditch the v1/v2 switcher, we can // be a bit more clever and move this. diff --git a/ee/localserver/krypto-ec-middleware_test.go b/ee/localserver/krypto-ec-middleware_test.go index ba659da1c..6f7c5acb0 100644 --- a/ee/localserver/krypto-ec-middleware_test.go +++ b/ee/localserver/krypto-ec-middleware_test.go @@ -21,8 +21,11 @@ import ( "github.com/kolide/krypto/pkg/challenge" "github.com/kolide/krypto/pkg/echelper" "github.com/kolide/launcher/ee/agent/keys" + "github.com/kolide/launcher/ee/localserver/mocks" + "github.com/kolide/launcher/pkg/log/multislogger" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -37,8 +40,7 @@ func TestKryptoEcMiddleware(t *testing.T) { koldieSessionId := ulid.New() cmdRequestHeaders := map[string][]string{ - "some_key": {ulid.New()}, - "other_key": {ulid.New()}, + kolidePresenceDetectionInterval: {"0s"}, } cmdReqCallBackHeaders := map[string][]string{ @@ -181,8 +183,15 @@ func TestKryptoEcMiddleware(t *testing.T) { kryptoEcMiddleware := newKryptoEcMiddleware(slogger, tt.localDbKey, tt.hardwareKey, counterpartyKey.PublicKey) require.NoError(t, err) + mockPresenceDetector := mocks.NewPresenceDetector(t) + mockPresenceDetector.On("DetectPresence", mock.AnythingOfType("string"), mock.AnythingOfType("Duration")).Return(0*time.Second, nil).Maybe() + localServer := &localServer{ + presenceDetector: mockPresenceDetector, + slogger: multislogger.NewNopLogger(), + } + // give our middleware with the test handler to the determiner - h := kryptoEcMiddleware.Wrap(testHandler) + h := kryptoEcMiddleware.Wrap(localServer.presenceDetectionHandler(testHandler)) rr := httptest.NewRecorder() h.ServeHTTP(rr, req) @@ -201,6 +210,7 @@ func TestKryptoEcMiddleware(t *testing.T) { require.NotEmpty(t, rr.Body.String()) require.Equal(t, kolideKryptoEccHeader20230130Value, rr.Header().Get(kolideKryptoHeaderKey)) + require.Equal(t, (0 * time.Second).String(), rr.Header().Get(kolideDurationSinceLastPresenceDetection)) // try to open the response returnedResponseBytes, err := base64.StdEncoding.DecodeString(rr.Body.String()) diff --git a/ee/localserver/server.go b/ee/localserver/server.go index e2a9620dc..2ce4e6464 100644 --- a/ee/localserver/server.go +++ b/ee/localserver/server.go @@ -111,13 +111,13 @@ func New(ctx context.Context, k types.Knapsack, presenceDetector presenceDetecto mux := http.NewServeMux() mux.HandleFunc("/", http.NotFound) - mux.Handle("/v0/cmd", ecKryptoMiddleware.Wrap(ecAuthedMux)) + mux.Handle("/v0/cmd", ecKryptoMiddleware.Wrap(ls.presenceDetectionHandler(ecAuthedMux))) // /v1/cmd was added after fixing a bug where local server would panic when an endpoint was not found // after making it through the kryptoEcMiddleware // by using v1, k2 can call endpoints without fear of panicing local server // /v0/cmd left for transition period - mux.Handle("/v1/cmd", ecKryptoMiddleware.Wrap(ecAuthedMux)) + mux.Handle("/v1/cmd", ecKryptoMiddleware.Wrap(ls.presenceDetectionHandler(ecAuthedMux))) // uncomment to test without going through middleware // for example: @@ -135,7 +135,7 @@ func New(ctx context.Context, k types.Knapsack, presenceDetector presenceDetecto ls.requestLoggingHandler( ls.preflightCorsHandler( ls.rateLimitHandler( - ls.presenceDetectionHandler(mux), + mux, ), )), "localserver", otelhttp.WithSpanNameFormatter(func(operation string, r *http.Request) string { return r.URL.Path @@ -426,6 +426,7 @@ func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler // no presence detection requested if detectionIntervalStr == "" { next.ServeHTTP(w, r) + return } detectionIntervalDuration, err := time.ParseDuration(detectionIntervalStr) From 98c8c51944a7c49db6f549455b16135523c24a88 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Fri, 27 Sep 2024 10:53:36 -0700 Subject: [PATCH 22/23] feedback --- ee/desktop/user/client/client.go | 1 + ee/presencedetection/presencedetection.go | 30 ++++++++----------- .../presencedetection_darwin_test.go | 6 ++-- .../presencedetection_test.go | 5 ++-- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/ee/desktop/user/client/client.go b/ee/desktop/user/client/client.go index 935ce9e71..da90291dd 100644 --- a/ee/desktop/user/client/client.go +++ b/ee/desktop/user/client/client.go @@ -67,6 +67,7 @@ func (c *client) DetectPresence(reason string, interval time.Duration) (time.Dur encodedReason := url.QueryEscape(reason) encodedInterval := url.QueryEscape(interval.String()) + // default time out of 30s is set in New() resp, requestErr := c.base.Get(fmt.Sprintf("http://unix/detect_presence?reason=%s&interval=%s", encodedReason, encodedInterval)) if requestErr != nil { return presencedetection.DetectionFailedDurationValue, fmt.Errorf("getting presence: %w", requestErr) diff --git a/ee/presencedetection/presencedetection.go b/ee/presencedetection/presencedetection.go index 95f3abe55..7a77d1042 100644 --- a/ee/presencedetection/presencedetection.go +++ b/ee/presencedetection/presencedetection.go @@ -9,8 +9,8 @@ import ( const DetectionFailedDurationValue = -1 * time.Second type PresenceDetector struct { - lastDetectionUTC time.Time - mutext sync.Mutex + lastDetection time.Time + mutext sync.Mutex // detector is an interface to allow for mocking in tests detector detectorIface } @@ -36,27 +36,23 @@ func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time pd.detector = &detector{} } - hadHadSuccessfulDetection := pd.lastDetectionUTC != time.Time{} - // Check if the last detection was within the detection interval - if hadHadSuccessfulDetection && time.Since(pd.lastDetectionUTC) < detectionInterval { - return time.Since(pd.lastDetectionUTC), nil + if (pd.lastDetection != time.Time{}) && time.Since(pd.lastDetection) < detectionInterval { + return time.Since(pd.lastDetection), nil } success, err := pd.detector.Detect(reason) - - switch { - case err != nil && hadHadSuccessfulDetection: - return time.Since(pd.lastDetectionUTC), fmt.Errorf("detecting presence: %w", err) - - case err != nil: // error without initial successful detection + if err != nil { + // if we got an error, we behave as if there have been no successful detections in the past return DetectionFailedDurationValue, fmt.Errorf("detecting presence: %w", err) + } - case success: - pd.lastDetectionUTC = time.Now().UTC() + if success { + pd.lastDetection = time.Now().UTC() return 0, nil - - default: // failed detection without error, maybe not possible? - return time.Since(pd.lastDetectionUTC), fmt.Errorf("detection failed without error") } + + // if we got here it means we failed without an error + // this "should" never happen, but here for completeness + return DetectionFailedDurationValue, fmt.Errorf("detection failed without OS error") } diff --git a/ee/presencedetection/presencedetection_darwin_test.go b/ee/presencedetection/presencedetection_darwin_test.go index 4241316dd..ec528ce02 100644 --- a/ee/presencedetection/presencedetection_darwin_test.go +++ b/ee/presencedetection/presencedetection_darwin_test.go @@ -8,14 +8,14 @@ import ( "github.com/stretchr/testify/require" ) -const testPresenceEnvVar = "launcher_test_presence" +const testPresenceEnvVar = "LAUNCHER_TEST_PRESENCE" // Since there is no way to test user presence in a CI / automated fashion, // these test are expected to be run manually via cmd line when needed. // To test this run // -// launcher_test_presence=true go test ./ee/presencedetection/ -run Test_detectSuccess +// LAUNCHER_TEST_PRESENCE=true go test ./ee/presencedetection/ -run Test_detectSuccess // // then successfully auth with the pop up func Test_detectSuccess(t *testing.T) { @@ -32,7 +32,7 @@ func Test_detectSuccess(t *testing.T) { // To test this run // -// launcher_test_presence=true go test ./ee/presencedetection/ -run Test_detectCancel +// LAUNCHER_TEST_PRESENCE=true go test ./ee/presencedetection/ -run Test_detectCancel // // then cancel the biometric auth that pops up func Test_detectCancel(t *testing.T) { diff --git a/ee/presencedetection/presencedetection_test.go b/ee/presencedetection/presencedetection_test.go index 6ed82f2fb..e8dbb603d 100644 --- a/ee/presencedetection/presencedetection_test.go +++ b/ee/presencedetection/presencedetection_test.go @@ -71,6 +71,7 @@ func TestPresenceDetector_DetectPresence(t *testing.T) { expectError: true, }, { + // this should never happen, but it is here for completeness name: "detection failed without OS error", interval: 0, detector: func(t *testing.T) detectorIface { @@ -89,8 +90,8 @@ func TestPresenceDetector_DetectPresence(t *testing.T) { t.Parallel() pd := &PresenceDetector{ - detector: tt.detector(t), - lastDetectionUTC: tt.initialLastDetectionUTC, + detector: tt.detector(t), + lastDetection: tt.initialLastDetectionUTC, } timeSinceLastDetection, err := pd.DetectPresence("this is a test", tt.interval) From 4d952b24b999f96aa008d490b3cc01d160a5e646 Mon Sep 17 00:00:00 2001 From: James Pickett Date: Fri, 27 Sep 2024 11:05:16 -0700 Subject: [PATCH 23/23] make presence detection part of test darwin only --- ee/localserver/krypto-ec-middleware_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ee/localserver/krypto-ec-middleware_test.go b/ee/localserver/krypto-ec-middleware_test.go index 6f7c5acb0..b562c073e 100644 --- a/ee/localserver/krypto-ec-middleware_test.go +++ b/ee/localserver/krypto-ec-middleware_test.go @@ -13,6 +13,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "runtime" "strings" "testing" "time" @@ -210,7 +211,10 @@ func TestKryptoEcMiddleware(t *testing.T) { require.NotEmpty(t, rr.Body.String()) require.Equal(t, kolideKryptoEccHeader20230130Value, rr.Header().Get(kolideKryptoHeaderKey)) - require.Equal(t, (0 * time.Second).String(), rr.Header().Get(kolideDurationSinceLastPresenceDetection)) + + if runtime.GOOS == "darwin" { + require.Equal(t, (0 * time.Second).String(), rr.Header().Get(kolideDurationSinceLastPresenceDetection)) + } // try to open the response returnedResponseBytes, err := base64.StdEncoding.DecodeString(rr.Body.String())