hardware key runners (#1977)

cmd/launcher/launcher.go

@@ -357,7 +357,7 @@ func runLauncher(ctx context.Context, cancel func(), multiSlogger, systemMultiSl
 	if err := osquery.SetupLauncherKeys(k.ConfigStore()); err != nil {
 		return fmt.Errorf("setting up initial launcher keys: %w", err)
 	}
-	if err := agent.SetupKeys(ctx, k.Slogger(), k.ConfigStore(), false); err != nil {
+	if err := agent.SetupKeys(ctx, k.Slogger(), k.ConfigStore()); err != nil {
 		return fmt.Errorf("setting up agent keys: %w", err)
 	}
 
@@ -419,6 +419,12 @@ func runLauncher(ctx context.Context, cancel func(), multiSlogger, systemMultiSl
 			return fmt.Errorf("failed to create desktop runner: %w", err)
 		}
 
+		execute, interrupt, err := agent.SetHardwareKeysRunner(ctx, k.Slogger(), k.ConfigStore(), runner)
+		if err != nil {
+			return fmt.Errorf("setting up hardware keys: %w", err)
+		}
+		runGroup.Add("hardwareKeys", execute, interrupt)
+
 		runGroup.Add("desktopRunner", runner.Execute, runner.Interrupt)
 		controlService.RegisterConsumer(desktopMenuSubsystemName, runner)
 

cmd/launcher/main.go

@@ -214,8 +214,6 @@ func runSubcommands(systemMultiSlogger *multislogger.MultiSlogger) error {
 		run = runDownloadOsquery
 	case "uninstall":
 		run = runUninstall
-	case "secure-enclave":
-		run = runSecureEnclave
 	case "watchdog": // note: this is currently only implemented for windows
 		run = watchdog.RunWatchdogTask
 	default:

ee/agent/keys.go

@@ -3,14 +3,12 @@ package agent
 import (
 	"context"
 	"crypto"
+	"crypto/ecdsa"
 	"fmt"
 	"log/slog"
-	"time"
 
 	"github.com/kolide/launcher/ee/agent/keys"
 	"github.com/kolide/launcher/ee/agent/types"
-	"github.com/kolide/launcher/pkg/backoff"
-	"github.com/kolide/launcher/pkg/traces"
 )
 
 type keyInt interface {
@@ -21,6 +19,7 @@ type keyInt interface {
 var hardwareKeys keyInt = keys.Noop
 var localDbKeys keyInt = keys.Noop
 
+// HardwareKeys returns the hardware keys for the agent, it's critical to not cache this value as it may change during runtime.
 func HardwareKeys() keyInt {
 	return hardwareKeys
 }
@@ -29,10 +28,11 @@ func LocalDbKeys() keyInt {
 	return localDbKeys
 }
 
-func SetupKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter, skipHardwareKeys bool) error {
-	ctx, span := traces.StartSpan(ctx)
-	defer span.End()
+type secureEnclaveClient interface {
+	CreateSecureEnclaveKey(uid string) (*ecdsa.PublicKey, error)
+}
 
+func SetupKeys(_ context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) error {
 	slogger = slogger.With("component", "agentkeys")
 
 	var err error
@@ -43,75 +43,5 @@ func SetupKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSett
 		return fmt.Errorf("setting up local db keys: %w", err)
 	}
 
-	if skipHardwareKeys {
-		return nil
-	}
-
-	err = backoff.WaitFor(func() error {
-		hwKeys, err := setupHardwareKeys(ctx, slogger, store)
-		if err != nil {
-			return err
-		}
-		hardwareKeys = hwKeys
-		return nil
-	}, 1*time.Second, 250*time.Millisecond)
-
-	if err != nil {
-		// Use of hardware keys is not fully implemented as of 2023-02-01, so log an error and move on
-		slogger.Log(context.TODO(), slog.LevelInfo,
-			"failed setting up hardware keys",
-			"err", err,
-		)
-	}
-
-	return nil
-}
-
-// This duplicates some of pkg/osquery/extension.go but that feels like the wrong place.
-// Really, we should have a simpler interface over a storage layer.
-const (
-	privateEccData = "privateEccData" // nolint:unused
-	publicEccData  = "publicEccData"  // nolint:unused
-)
-
-// nolint:unused
-func fetchKeyData(store types.Getter) ([]byte, []byte, error) {
-	pri, err := store.Get([]byte(privateEccData))
-	if err != nil {
-		return nil, nil, err
-	}
-
-	pub, err := store.Get([]byte(publicEccData))
-	if err != nil {
-		return nil, nil, err
-	}
-
-	return pri, pub, nil
-}
-
-// nolint:unused
-func storeKeyData(store types.Setter, pri, pub []byte) error {
-	if pri != nil {
-		if err := store.Set([]byte(privateEccData), pri); err != nil {
-			return err
-		}
-	}
-
-	if pub != nil {
-		if err := store.Set([]byte(publicEccData), pub); err != nil {
-			return err
-		}
-	}
-
 	return nil
 }
-
-// clearKeyData is used to clear the keys as part of error handling around new keys. It is not intended to be called
-// regularly, and since the path that calls it is around DB errors, it has no error handling.
-// nolint:unused
-func clearKeyData(slogger *slog.Logger, deleter types.Deleter) {
-	slogger.Log(context.TODO(), slog.LevelInfo,
-		"clearing keys",
-	)
-	_ = deleter.Delete([]byte(privateEccData), []byte(publicEccData))
-}

ee/agent/keys_darwin.go

@@ -9,19 +9,22 @@ import (
 	"log/slog"
 
 	"github.com/kolide/launcher/ee/agent/types"
-	"github.com/kolide/launcher/ee/secureenclavesigner"
+	"github.com/kolide/launcher/ee/secureenclaverunner"
 	"github.com/kolide/launcher/pkg/traces"
 )
 
-func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) {
+// SetHardwareKeysRunner creates a secure enclave runner and sets it as the agent hardware key as it also implements the keyInt/crypto.Signer interface.
+// The returned execute and interrupt functions can be used to start and stop the secure enclave runner, generally via a run group.
+func SetHardwareKeysRunner(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter, secureEnclaveClient secureEnclaveClient) (execute func() error, interrupt func(error), err error) {
 	ctx, span := traces.StartSpan(ctx)
 	defer span.End()
 
-	ses, err := secureenclavesigner.New(ctx, slogger, store)
+	ser, err := secureenclaverunner.New(ctx, slogger, store, secureEnclaveClient)
 	if err != nil {
 		traces.SetError(span, fmt.Errorf("creating secureenclave signer: %w", err))
-		return nil, fmt.Errorf("creating secureenclave signer: %w", err)
+		return nil, nil, fmt.Errorf("creating secureenclave signer: %w", err)
 	}
 
-	return ses, nil
+	hardwareKeys = ser
+	return ser.Execute, ser.Interrupt, nil
 }