revert revert of secure enclave keys (#1651)

cmd/launcher/main.go

@@ -159,6 +159,8 @@ func runSubcommands() error {
 		run = runDownloadOsquery
 	case "uninstall":
 		run = runUninstall
+	case "secure-enclave":
+		run = runSecureEnclave
 	default:
 		return fmt.Errorf("unknown subcommand %s", os.Args[1])
 	}

cmd/launcher/secure_enclave_darwin.go

@@ -0,0 +1,48 @@
+//go:build darwin
+// +build darwin
+
+package main
+
+import (
+	"errors"
+	"fmt"
+	"os"
+
+	"github.com/kolide/krypto/pkg/echelper"
+	"github.com/kolide/krypto/pkg/secureenclave"
+	"github.com/kolide/launcher/ee/secureenclavesigner"
+)
+
+// runSecureEnclave performs either a create-key operation using the secure enclave.
+// It's available as a separate command because launcher runs as root by default and since it's
+// not in a user security context, it can't use the secure enclave directly. However, this command
+// can be run in the user context using launchctl.
+func runSecureEnclave(args []string) error {
+	// currently we are just creating key, but plan to add sign command in future
+	if len(args) < 1 {
+		return errors.New("not enough arguments, expect create_key")
+	}
+
+	switch args[0] {
+	case secureenclavesigner.CreateKeyCmd:
+		return createSecureEnclaveKey()
+
+	default:
+		return fmt.Errorf("unknown command %s", args[0])
+	}
+}
+
+func createSecureEnclaveKey() error {
+	secureEnclavePubKey, err := secureenclave.CreateKey()
+	if err != nil {
+		return fmt.Errorf("creating secure enclave key: %w", err)
+	}
+
+	secureEnclavePubDer, err := echelper.PublicEcdsaToB64Der(secureEnclavePubKey)
+	if err != nil {
+		return fmt.Errorf("marshalling public key to der: %w", err)
+	}
+
+	os.Stdout.Write(secureEnclavePubDer)
+	return nil
+}

cmd/launcher/secure_enclave_other.go

@@ -0,0 +1,10 @@
+//go:build !darwin
+// +build !darwin
+
+package main
+
+import "errors"
+
+func runSecureEnclave(args []string) error {
+	return errors.New("not implemented on non darwin platforms")
+}

cmd/launcher/secure_enclave_test.go

@@ -0,0 +1,148 @@
+//go:build darwin
+// +build darwin
+
+package main
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/kolide/krypto/pkg/echelper"
+	"github.com/kolide/launcher/ee/secureenclavesigner"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	testWrappedEnvVarKey = "SECURE_ENCLAVE_TEST_WRAPPED"
+	macOsAppResourceDir  = "../../ee/secureenclavesigner/test_app_resources"
+)
+
+// TestSecureEnclaveTestRunner creates a MacOS app with the binary of this packages tests, then signs the app with entitlements and runs the tests.
+// This is done because in order to access secure enclave to run tests, we need MacOS entitlements.
+// #nosec G306 -- Need readable files
+func TestSecureEnclaveTestRunner(t *testing.T) {
+	t.Parallel()
+
+	if os.Getenv("CI") != "" {
+		t.Skipf("\nskipping because %s env var was not empty, this is being run in a CI environment without access to secure enclave", testWrappedEnvVarKey)
+	}
+
+	if os.Getenv(testWrappedEnvVarKey) != "" {
+		t.Skipf("\nskipping because %s env var was not empty, this is the execution of the codesigned app with entitlements", testWrappedEnvVarKey)
+	}
+
+	t.Log("\nexecuting wrapped tests with codesigned app and entitlements")
+
+	// set up app bundle
+	rootDir := t.TempDir()
+	appRoot := filepath.Join(rootDir, "launcher_test.app")
+
+	// make required dirs launcher_test.app/Contents/MacOS and add files
+	require.NoError(t, os.MkdirAll(filepath.Join(appRoot, "Contents", "MacOS"), 0700))
+	copyFile(t, filepath.Join(macOsAppResourceDir, "Info.plist"), filepath.Join(appRoot, "Contents", "Info.plist"))
+	copyFile(t, filepath.Join(macOsAppResourceDir, "embedded.provisionprofile"), filepath.Join(appRoot, "Contents", "embedded.provisionprofile"))
+
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+
+	// build an executable containing the tests into the app bundle
+	executablePath := filepath.Join(appRoot, "Contents", "MacOS", "launcher_test")
+	out, err := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd
+		ctx,
+		"go",
+		"test",
+		"-c",
+		"--cover",
+		"--race",
+		"./",
+		"-o",
+		executablePath,
+	).CombinedOutput()
+
+	require.NoError(t, ctx.Err())
+	require.NoError(t, err, string(out))
+
+	// sign app bundle
+	signApp(t, appRoot)
+
+	// run app bundle executable
+	cmd := exec.CommandContext(ctx, executablePath, "-test.v") //nolint:forbidigo // Only used in test, don't want as standard allowedcmd
+	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", testWrappedEnvVarKey, "true"))
+	out, err = cmd.CombinedOutput()
+	require.NoError(t, ctx.Err())
+	require.NoError(t, err, string(out))
+
+	// ensure the test ran successfully
+	require.Contains(t, string(out), "PASS: TestSecureEnclaveCmd")
+	require.NotContains(t, string(out), "FAIL")
+}
+
+func TestSecureEnclaveCmd(t *testing.T) { //nolint:paralleltest
+	if os.Getenv(testWrappedEnvVarKey) == "" {
+		t.Skipf("\nskipping because %s env var was empty, test not being run from codesigned app with entitlements", testWrappedEnvVarKey)
+	}
+
+	t.Log("\nrunning wrapped tests with codesigned app and entitlements")
+
+	oldStdout := os.Stdout
+	defer func() {
+		os.Stdout = oldStdout
+	}()
+
+	// create a pipe to capture stdout
+	pipeReader, pipeWriter, err := os.Pipe()
+	require.NoError(t, err)
+
+	os.Stdout = pipeWriter
+
+	require.NoError(t, runSecureEnclave([]string{secureenclavesigner.CreateKeyCmd}))
+	require.NoError(t, pipeWriter.Close())
+
+	var buf bytes.Buffer
+	_, err = buf.ReadFrom(pipeReader)
+	require.NoError(t, err)
+
+	// convert response to public key
+	createKeyResponse := buf.Bytes()
+	secureEnclavePubKey, err := echelper.PublicB64DerToEcdsaKey(createKeyResponse)
+	require.NoError(t, err)
+	require.NotNil(t, secureEnclavePubKey, "should be able to get public key")
+}
+
+// #nosec G306 -- Need readable files
+func copyFile(t *testing.T, source, destination string) {
+	bytes, err := os.ReadFile(source)
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(destination, bytes, 0700))
+}
+
+// #nosec G204 -- This triggers due to using env var in cmd, making exception for test
+func signApp(t *testing.T, appRootDir string) {
+	codeSignId := os.Getenv("MACOS_CODESIGN_IDENTITY")
+	require.NotEmpty(t, codeSignId, "need MACOS_CODESIGN_IDENTITY env var to sign app, such as [Mac Developer: Jane Doe (ABCD123456)]")
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	cmd := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd
+		ctx,
+		"codesign",
+		"--deep",
+		"--force",
+		"--options", "runtime",
+		"--entitlements", filepath.Join(macOsAppResourceDir, "entitlements"),
+		"--sign", codeSignId,
+		"--timestamp",
+		appRootDir,
+	)
+
+	out, err := cmd.CombinedOutput()
+	require.NoError(t, ctx.Err())
+	require.NoError(t, err, string(out))
+}

ee/agent/keys.go

@@ -5,12 +5,12 @@ import (
 	"crypto"
 	"fmt"
 	"log/slog"
-	"runtime"
 	"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 {
@@ -29,7 +29,10 @@ func LocalDbKeys() keyInt {
 	return localDbKeys
 }
 
-func SetupKeys(slogger *slog.Logger, store types.GetterSetterDeleter) error {
+func SetupKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter, skipHardwareKeys bool) error {
+	ctx, span := traces.StartSpan(ctx)
+	defer span.End()
+
 	slogger = slogger.With("component", "agentkeys")
 
 	var err error
@@ -40,13 +43,12 @@ func SetupKeys(slogger *slog.Logger, store types.GetterSetterDeleter) error {
 		return fmt.Errorf("setting up local db keys: %w", err)
 	}
 
-	// Secure Enclave is not currently supported, so don't spend startup time waiting for it to work -- see keys_darwin.go for more details.
-	if runtime.GOOS == "darwin" {
+	if skipHardwareKeys {
 		return nil
 	}
 
 	err = backoff.WaitFor(func() error {
-		hwKeys, err := setupHardwareKeys(slogger, store)
+		hwKeys, err := setupHardwareKeys(ctx, slogger, store)
 		if err != nil {
 			return err
 		}

ee/agent/keys_darwin.go

@@ -4,46 +4,24 @@
 package agent
 
 import (
-	"errors"
+	"context"
+	"fmt"
 	"log/slog"
 
 	"github.com/kolide/launcher/ee/agent/types"
+	"github.com/kolide/launcher/ee/secureenclavesigner"
+	"github.com/kolide/launcher/pkg/traces"
 )
 
-// nolint:unused
-func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) {
-	// We're seeing issues where launcher hangs (and does not complete startup) on the
-	// Sonoma Beta 2 release when trying to interact with the secure enclave below, on
-	// CreateKey. Since we don't expect this to work at the moment anyway, we are
-	// short-circuiting and returning early for now.
-	return nil, errors.New("secure enclave is not currently supported")
+func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) {
+	ctx, span := traces.StartSpan(ctx)
+	defer span.End()
 
-	/*
-		_, pubData, err := fetchKeyData(store)
-		if err != nil {
-			return nil, err
-		}
+	ses, err := secureenclavesigner.New(ctx, slogger, store)
+	if err != nil {
+		traces.SetError(span, fmt.Errorf("creating secureenclave signer: %w", err))
+		return nil, fmt.Errorf("creating secureenclave signer: %w", err)
+	}
 
-		if pubData == nil {
-			level.Info(logger).Log("msg", "Generating new keys")
-
-			var err error
-			pubData, err = secureenclave.CreateKey()
-			if err != nil {
-				return nil, fmt.Errorf("creating key: %w", err)
-			}
-
-			if err := storeKeyData(store, nil, pubData); err != nil {
-				clearKeyData(logger, store)
-				return nil, fmt.Errorf("storing key: %w", err)
-			}
-		}
-
-		k, err := secureenclave.New(pubData)
-		if err != nil {
-			return nil, fmt.Errorf("creating secureenclave signer: %w", err)
-		}
-
-		return k, nil
-	*/
+	return ses, nil
 }

ee/agent/keys_tpm.go

@@ -10,10 +10,13 @@ import (
 
 	"github.com/kolide/krypto/pkg/tpm"
 	"github.com/kolide/launcher/ee/agent/types"
+	"github.com/kolide/launcher/pkg/traces"
 )
 
-// nolint:unused
-func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) {
+func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) {
+	_, span := traces.StartSpan(ctx)
+	defer span.End()
+
 	priData, pubData, err := fetchKeyData(store)
 	if err != nil {
 		return nil, err
@@ -28,17 +31,24 @@ func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (k
 		priData, pubData, err = tpm.CreateKey()
 		if err != nil {
 			clearKeyData(slogger, store)
+			traces.SetError(span, fmt.Errorf("creating key: %w", err))
 			return nil, fmt.Errorf("creating key: %w", err)
 		}
 
+		span.AddEvent("new_key_created")
+
 		if err := storeKeyData(store, priData, pubData); err != nil {
 			clearKeyData(slogger, store)
+			traces.SetError(span, fmt.Errorf("storing key: %w", err))
 			return nil, fmt.Errorf("storing key: %w", err)
 		}
+
+		span.AddEvent("new_key_stored")
 	}
 
 	k, err := tpm.New(priData, pubData)
 	if err != nil {
+		traces.SetError(span, fmt.Errorf("creating tpm signer: from new key: %w", err))
 		return nil, fmt.Errorf("creating tpm signer: from new key: %w", err)
 	}
 

ee/control/client_http.go

@@ -11,6 +11,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"runtime"
 	"time"
 
 	"github.com/kolide/krypto/pkg/echelper"
@@ -97,7 +98,9 @@ func (c *HTTPClient) GetConfig() (io.Reader, error) {
 
 	// Calculate second signature if available
 	hardwareKeys := agent.HardwareKeys()
-	if hardwareKeys.Public() != nil {
+
+	// hardware signing is not implemented for darwin
+	if runtime.GOOS != "darwin" && hardwareKeys.Public() != nil {
 		key2, err := echelper.PublicEcdsaToB64Der(hardwareKeys.Public().(*ecdsa.PublicKey))
 		if err != nil {
 			return nil, fmt.Errorf("could not get key header from hardware keys: %w", err)