From bb7805d97b9e88bd7fcb6c2faf06af74584de166 Mon Sep 17 00:00:00 2001 From: james pickett Date: Thu, 7 Mar 2024 10:52:55 -0800 Subject: [PATCH 01/13] add support for collecitng multiple user keys on via secure enclave --- cmd/launcher/main.go | 2 + cmd/launcher/secure_enclave_darwin.go | 48 ++++ cmd/launcher/secure_enclave_other.go | 10 + cmd/launcher/secure_enclave_test.go | 148 +++++++++++ ee/agent/keys.go | 6 - ee/agent/keys_darwin.go | 71 +++--- ee/secureenclavesigner/mocks/persister.go | 38 +++ .../secureenclavesigner_darwin.go | 240 ++++++++++++++++++ .../secureenclavesigner_test.go | 176 +++++++++++++ .../embedded.provisionprofile | Bin 0 -> 12562 bytes .../test_app_resources/entitlements | 8 + .../test_app_resources/info.plist | 20 ++ .../test_app_resources/readme.md | 25 ++ pkg/osquery/table/launcher_info.go | 13 + 14 files changed, 770 insertions(+), 35 deletions(-) create mode 100644 cmd/launcher/secure_enclave_darwin.go create mode 100644 cmd/launcher/secure_enclave_other.go create mode 100644 cmd/launcher/secure_enclave_test.go create mode 100644 ee/secureenclavesigner/mocks/persister.go create mode 100644 ee/secureenclavesigner/secureenclavesigner_darwin.go create mode 100644 ee/secureenclavesigner/secureenclavesigner_test.go create mode 100644 ee/secureenclavesigner/test_app_resources/embedded.provisionprofile create mode 100644 ee/secureenclavesigner/test_app_resources/entitlements create mode 100644 ee/secureenclavesigner/test_app_resources/info.plist create mode 100644 ee/secureenclavesigner/test_app_resources/readme.md diff --git a/cmd/launcher/main.go b/cmd/launcher/main.go index 422cf7e55..78edaae23 100644 --- a/cmd/launcher/main.go +++ b/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]) } diff --git a/cmd/launcher/secure_enclave_darwin.go b/cmd/launcher/secure_enclave_darwin.go new file mode 100644 index 000000000..4475ffe57 --- /dev/null +++ b/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 or sign 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 +} diff --git a/cmd/launcher/secure_enclave_other.go b/cmd/launcher/secure_enclave_other.go new file mode 100644 index 000000000..6192b77e4 --- /dev/null +++ b/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") +} diff --git a/cmd/launcher/secure_enclave_test.go b/cmd/launcher/secure_enclave_test.go new file mode 100644 index 000000000..debec4942 --- /dev/null +++ b/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)) +} diff --git a/ee/agent/keys.go b/ee/agent/keys.go index 23bb69637..22bddb340 100644 --- a/ee/agent/keys.go +++ b/ee/agent/keys.go @@ -5,7 +5,6 @@ import ( "crypto" "fmt" "log/slog" - "runtime" "time" "github.com/kolide/launcher/ee/agent/keys" @@ -40,11 +39,6 @@ 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" { - return nil - } - err = backoff.WaitFor(func() error { hwKeys, err := setupHardwareKeys(slogger, store) if err != nil { diff --git a/ee/agent/keys_darwin.go b/ee/agent/keys_darwin.go index 1f8fd4ecb..8df4db9ba 100644 --- a/ee/agent/keys_darwin.go +++ b/ee/agent/keys_darwin.go @@ -4,46 +4,59 @@ package agent import ( + "context" + "encoding/json" "errors" + "fmt" "log/slog" "github.com/kolide/launcher/ee/agent/types" + "github.com/kolide/launcher/ee/secureenclavesigner" ) -// 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") - - /* - _, pubData, err := fetchKeyData(store) - if err != nil { - return nil, err - } +// persister satisfies the persister interface for secureenclavesigner +type persister struct { + store types.GetterSetterDeleter +} - if pubData == nil { - level.Info(logger).Log("msg", "Generating new keys") +func (p *persister) Persist(data []byte) error { + return storeKeyData(p.store, nil, data) +} - var err error - pubData, err = secureenclave.CreateKey() - if err != nil { - return nil, fmt.Errorf("creating key: %w", err) - } +func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { - if err := storeKeyData(store, nil, pubData); err != nil { - clearKeyData(logger, store) - return nil, fmt.Errorf("storing key: %w", err) + // fetch any existing key data + _, pubData, err := fetchKeyData(store) + if err != nil { + return nil, err + } + + ses, err := secureenclavesigner.New(slogger, &persister{store: store}) + if err != nil { + return nil, fmt.Errorf("creating secureenclave signer: %w", err) + } + + if pubData != nil { + if err := json.Unmarshal(pubData, &ses); err != nil { + // data is corrupt or not in the expected format, clear it + slogger.Log(context.TODO(), slog.LevelError, + "could not unmarshal stored key data, clearing key data and generating new keys", + "err", err, + ) + clearKeyData(slogger, store) + + ses, err = secureenclavesigner.New(slogger, &persister{store: store}) + if err != nil { + return nil, fmt.Errorf("creating secureenclave signer: %w", err) } } + } - k, err := secureenclave.New(pubData) - if err != nil { - return nil, fmt.Errorf("creating secureenclave signer: %w", err) - } + // this is kind of weird, but we need to call public to ensure the key is generated + // it's done this way to do satisfying signer interface which doesn't return an error + if ses.Public() == nil { + return nil, errors.New("public key was not be created") + } - return k, nil - */ + return ses, nil } diff --git a/ee/secureenclavesigner/mocks/persister.go b/ee/secureenclavesigner/mocks/persister.go new file mode 100644 index 000000000..7a96082c9 --- /dev/null +++ b/ee/secureenclavesigner/mocks/persister.go @@ -0,0 +1,38 @@ +// Code generated by mockery v2.33.3. DO NOT EDIT. + +package mocks + +import mock "github.com/stretchr/testify/mock" + +// Persister is an autogenerated mock type for the persister type +type Persister struct { + mock.Mock +} + +// Persist provides a mock function with given fields: _a0 +func (_m *Persister) Persist(_a0 []byte) error { + ret := _m.Called(_a0) + + var r0 error + if rf, ok := ret.Get(0).(func([]byte) error); ok { + r0 = rf(_a0) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// NewPersister creates a new instance of Persister. 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 NewPersister(t interface { + mock.TestingT + Cleanup(func()) +}) *Persister { + mock := &Persister{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/ee/secureenclavesigner/secureenclavesigner_darwin.go b/ee/secureenclavesigner/secureenclavesigner_darwin.go new file mode 100644 index 000000000..c6a7940ff --- /dev/null +++ b/ee/secureenclavesigner/secureenclavesigner_darwin.go @@ -0,0 +1,240 @@ +//go:build darwin +// +build darwin + +package secureenclavesigner + +import ( + "context" + "crypto" + "crypto/ecdsa" + "encoding/json" + "fmt" + "io" + "log/slog" + "os" + "os/user" + "strings" + "sync" + + "github.com/kolide/krypto/pkg/echelper" + "github.com/kolide/launcher/ee/allowedcmd" + "github.com/kolide/launcher/ee/consoleuser" +) + +const ( + CreateKeyCmd = "create-key" +) + +type opt func(*SecureEnclaveSigner) + +type SecureEnclaveSigner struct { + uidPubKeyMap map[string]*ecdsa.PublicKey + pathToLauncherBinary string + persister persister + slogger *slog.Logger + mux *sync.Mutex +} + +// persister is an interface that allows the secure enclave signer to persist +// keys it creates, this is needed since we may need to create a new key anytime +// a new user logs in on macos +type persister interface { + Persist([]byte) error +} + +func New(slogger *slog.Logger, persister persister, opts ...opt) (*SecureEnclaveSigner, error) { + ses := &SecureEnclaveSigner{ + uidPubKeyMap: make(map[string]*ecdsa.PublicKey), + persister: persister, + slogger: slogger.With("component", "secureenclavesigner"), + mux: &sync.Mutex{}, + } + + for _, opt := range opts { + opt(ses) + } + + if ses.pathToLauncherBinary == "" { + p, err := os.Executable() + if err != nil { + return nil, fmt.Errorf("getting path to launcher binary: %w", err) + } + + ses.pathToLauncherBinary = p + } + + return ses, nil +} + +// Public returns the public key of the current console user +// creating and peristing a new one if needed +func (ses *SecureEnclaveSigner) Public() crypto.PublicKey { + ses.mux.Lock() + defer ses.mux.Unlock() + + c, err := firstConsoleUser() + if err != nil { + ses.slogger.Log(context.TODO(), slog.LevelError, + "getting first console user", + "err", err, + ) + + return nil + } + + if v, ok := ses.uidPubKeyMap[c.Uid]; ok { + return v + } + + key, err := ses.createKey(context.TODO(), c) + if err != nil { + ses.slogger.Log(context.TODO(), slog.LevelError, + "creating key", + "err", err, + ) + + return nil + } + + ses.uidPubKeyMap[c.Uid] = key + + // pesist the new key + json, err := json.Marshal(ses) + if err != nil { + ses.slogger.Log(context.TODO(), slog.LevelError, + "marshaling secure enclave signer", + "err", err, + ) + + delete(ses.uidPubKeyMap, c.Uid) + return nil + } + + if err := ses.persister.Persist(json); err != nil { + ses.slogger.Log(context.TODO(), slog.LevelError, + "persisting secure enclave signer", + "err", err, + ) + delete(ses.uidPubKeyMap, c.Uid) + return nil + } + + return key +} + +func (ses *SecureEnclaveSigner) Type() string { + return "secure_enclave" +} + +func (ses *SecureEnclaveSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + return nil, fmt.Errorf("not implemented") +} + +type keyData struct { + Uid string `json:"uid"` + PubKey string `json:"pub_key"` +} + +func (ses *SecureEnclaveSigner) MarshalJSON() ([]byte, error) { + var keyDatas []keyData + + for uid, pubKey := range ses.uidPubKeyMap { + pubKeyBytes, err := echelper.PublicEcdsaToB64Der(pubKey) + if err != nil { + return nil, fmt.Errorf("converting public key to b64 der: %w", err) + } + + keyDatas = append(keyDatas, keyData{ + Uid: uid, + PubKey: string(pubKeyBytes), + }) + + } + + return json.Marshal(keyDatas) +} + +func (ses *SecureEnclaveSigner) UnmarshalJSON(data []byte) error { + if ses.uidPubKeyMap == nil { + ses.uidPubKeyMap = make(map[string]*ecdsa.PublicKey) + } + + var keyDatas []keyData + if err := json.Unmarshal(data, &keyDatas); err != nil { + return fmt.Errorf("unmarshalling key data: %w", err) + } + + for _, kd := range keyDatas { + pubKey, err := echelper.PublicB64DerToEcdsaKey([]byte(kd.PubKey)) + if err != nil { + return fmt.Errorf("converting public key to ecdsa: %w", err) + } + + ses.uidPubKeyMap[kd.Uid] = pubKey + } + + return nil +} + +func (ses *SecureEnclaveSigner) createKey(ctx context.Context, u *user.User) (*ecdsa.PublicKey, error) { + cmd, err := allowedcmd.Launchctl( + ctx, + "asuser", + u.Uid, + "sudo", + "--preserve-env", + "-u", + u.Username, + ses.pathToLauncherBinary, + "secure-enclave", + CreateKeyCmd, + ) + + if err != nil { + return nil, fmt.Errorf("creating command to create key: %w", err) + } + + // skip updates since we have full path of binary + cmd.Env = append(cmd.Environ(), fmt.Sprintf("%s=%s", "LAUNCHER_SKIP_UPDATES", "true")) + out, err := cmd.CombinedOutput() + if err != nil { + return nil, fmt.Errorf("executing launcher binary to create key: %w: %s", err, string(out)) + } + + pubKey, err := echelper.PublicB64DerToEcdsaKey([]byte(lastLine(out))) + if err != nil { + return nil, fmt.Errorf("marshalling public key to der: %w", err) + } + + return pubKey, nil +} + +// lastLine returns the last line of the out. +// This is needed because laucher sets up a logger by default. +// The last line of the output is the public key or signature. +func lastLine(out []byte) string { + outStr := string(out) + + // get last line of outstr + lastLine := "" + for _, line := range strings.Split(outStr, "\n") { + if line != "" { + lastLine = line + } + } + + return lastLine +} + +func firstConsoleUser() (*user.User, error) { + c, err := consoleuser.CurrentUsers(context.TODO()) + if err != nil { + return nil, fmt.Errorf("getting current users: %w", err) + } + + if len(c) == 0 { + return nil, fmt.Errorf("no console users found") + } + + return c[0], nil +} diff --git a/ee/secureenclavesigner/secureenclavesigner_test.go b/ee/secureenclavesigner/secureenclavesigner_test.go new file mode 100644 index 000000000..88d32dce4 --- /dev/null +++ b/ee/secureenclavesigner/secureenclavesigner_test.go @@ -0,0 +1,176 @@ +//go:build darwin +// +build darwin + +package secureenclavesigner + +import ( + "context" + "crypto/ecdsa" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/kolide/krypto/pkg/echelper" + "github.com/kolide/launcher/ee/secureenclavesigner/mocks" + "github.com/kolide/launcher/pkg/log/multislogger" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +const ( + testWrappedEnvVarKey = "SECURE_ENCLAVE_TEST_WRAPPED" + macOsAppResourceDir = "./test_app_resources" +) + +func WithBinaryPath(p string) opt { + return func(ses *SecureEnclaveSigner) { + ses.pathToLauncherBinary = p + } +} + +// #nosec G306 -- Need readable files +func TestSecureEnclaveSigner(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) + } + + // set up app bundle + rootDir := "/tmp/secure_enclave_test" + appRoot := filepath.Join(rootDir, "launcher_test.app") + + // make required dirs krypto_test.app/Contents/MacOS and add files + require.NoError(t, os.MkdirAll(filepath.Join(appRoot, "Contents", "MacOS"), 0777)) + 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() + + serverPrivKey, err := echelper.GenerateEcdsaKey() + require.NoError(t, err) + + serverPubKeyDer, err := echelper.PublicEcdsaToB64Der(serverPrivKey.Public().(*ecdsa.PublicKey)) + require.NoError(t, err) + + // build the executable + 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", + "build", + "-ldflags", + fmt.Sprintf("-X github.com/kolide/launcher/ee/secureenclavesigner.TestServerPubKey=%s", string(serverPubKeyDer)), + "-tags", + "secure_enclave_test", + "-o", + executablePath, + "../../cmd/launcher", + ).CombinedOutput() + + require.NoError(t, ctx.Err()) + require.NoError(t, err, string(out)) + + // sign app bundle + signApp(t, appRoot) + + persister := mocks.NewPersister(t) + + // create brand new signer without existing key + // ask for public first to trigger key generation + ses, err := New(multislogger.NewNopLogger(), persister, WithBinaryPath(executablePath)) + require.NoError(t, err) + + // should only call persist once on first public key generation + persister.On("Persist", mock.Anything).Return(nil).Once() + + pubKey := ses.Public() + require.NotNil(t, pubKey, + "should be able to create brand new public key", + ) + + pubEcdsaKey := pubKey.(*ecdsa.PublicKey) + require.NotNil(t, pubEcdsaKey, + "public key should convert to ecdsa key", + ) + + pubKeySame := ses.Public() + require.NotNil(t, pubKeySame, + "should be able to get public key again", + ) + + pubEcdsaKeySame := pubKeySame.(*ecdsa.PublicKey) + require.NotNil(t, pubEcdsaKeySame, + "public key should convert to ecdsa key", + ) + + require.Equal(t, pubEcdsaKey, pubEcdsaKeySame, + "asking for the same public key should return the same key", + ) + + jsonBytes, err := json.Marshal(ses) + require.NoError(t, err, + "should be able to marshal secure enclave signer to json", + ) + + unmarshalledSes, err := New(multislogger.NewNopLogger(), persister, WithBinaryPath(executablePath)) + require.NoError(t, err) + + require.NoError(t, json.Unmarshal(jsonBytes, &unmarshalledSes), + "should be able to unmarshal secure enclave signer from json", + ) + + // use same test binary + unmarshalledSes.pathToLauncherBinary = ses.pathToLauncherBinary + + pubKeyUnmarshalled := unmarshalledSes.Public() + require.NotNil(t, pubKeyUnmarshalled, + "should be able to get public key from unmarshalled secure enclave signer", + ) + + pubEcdsaKeyUnmarshalled := pubKeyUnmarshalled.(*ecdsa.PublicKey) + require.NotNil(t, pubEcdsaKeyUnmarshalled, + "public key should convert to ecdsa key", + ) + + require.Equal(t, pubEcdsaKey, pubEcdsaKeyUnmarshalled, + "unmarshalled public key should be the same as original 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 allowcmd + 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)) +} diff --git a/ee/secureenclavesigner/test_app_resources/embedded.provisionprofile b/ee/secureenclavesigner/test_app_resources/embedded.provisionprofile new file mode 100644 index 0000000000000000000000000000000000000000..148e280811a0e817611b3e306c854bae046f6c3f GIT binary patch literal 12562 zcmdUV3AhvGxj*NyA2t<4L6LyFheIZNqJYkp$t0O1lgvyODw<4^$t08IWSb;bg`?sE zR%_8-6)7&Y)U7Hmb)#BsMXkG7K}%b=E?}+HmRelyJIPs5+x!1N_vyVi&ofVEzWL@` z-}m?a-uL~G0mIBmQ%_mzTDyNn*W~Wa1IQQ10CJ$atIIGAnL24=@67St^Cyl&1{Q7^ zsn)f5VBxhB1{Pku?DR%n(bZGsid-x#Uu;;4EY_t8VlgQfq~(iw&e>;QeEN!>Ws3+u z&P4;HuB6D7sxH7|UK-ae?$hhhQb|ea^#n)g0$w`A>7Wh0p7br&EzVS{rBn5KRaKY9 zwOUKXVqU9Q(Fe-KQmR~SdZDpC_+V+Wnp_OsbXp%X94Jl7Vs%B&)MdF;a|Luq6TWyp zwM?%)LTRO1hQY4jGAW&>sL08bjw~cgMY&Mbp;D?)U8WzbG17t}i}5NH-_7cm&@_djYk}I-WjyHP}Atv^uyJ(q2T3IyHO%u9T{8_9#$9;qqz?;_8Bak z!Fs9zIn`u|Ez`GO4UQ?G#H;CIIsc^r3{TG=ihO<7bR_~?mSW0rD)5J!^*>?VF^-|;krXpnYs+d6<@*7w6o0NIOYyZs7hm~M6;1>0&n%p{S;wt z1`1Zvo->B*1nR@2Tz@7fQw|luP`2Vk3CzIotjbDJLSR{fAgY+?!YZ*47D<@gm6$78 zic!22qy%Fu+^CC`(4?JiB|$lnc-T=Bn-~#`xZyrliW)m*H0d-XsZ3qWD@ckXy$qV8 zPy$TMG_BTZv;_TarCoha(ZQPz&a{$jY~ zfqN2d8tQ^g$>3$ZD50rDD3kTc?tr1j+T+bE@8T`E;+Iv{E5wi#frcfFb?YmrkPGDNF|=EasbA5_FF z+8EWQibkVK+!aJbqFDDD9a+(Y)}j$iO;CI-YIIZw%T%UmFeQz_W~{}izAVfwXx8L1 zNuwdShdq_ZpeA1g8+Ch%F*;LEW@9LiF|pSZEiP5eq>JO*vblNvOuOx?vy88AITG7Omf#=C9C_L$ zv|=HEf`i5)Qko9>v@x{5`x0ikuqKQNa17?8^%n^$iLg*hx|kY6h!l=jD3s-$7~rc@ zt`?K7penXt?M<*t2kqO~zJ*LO5-b9imXfYqox!5*^>?!*rdL^pL5ri>W(WpD+V2U8 z4P#iS+@H=Q7`%|;y+WvEHYuV(#Av0Q76{bemPgi(p=mpbp$v|hQI%Aqo{@cw0`|50 z!7&z9NA_6s0hV~7jDYP5(uX08crvx&3Z%1K&fSFsW_eTa$zOnYC0Gvu2fj56wmhO6mPxc zHPVdL+H~4H2|_RT8Yn+o^^u-@AdpK@WPwy%Mu`ZOF~sT%n31R@9e1ZGcN#HCOuCFk z3{)~3HOZ+!$s8zUk%rNmpc?v6Bi`~PTO~9m`&=P6N9j#!47YnYrJm6%SS~9&#R?X$ zAw{2Lj_Y%ArZ*FZi#r2rhOcN;{-2hmY_kjG>f4~nVf{h zScXIaZBTfRHh2mT^;01_VIo-4g~p*wjgpeY;7pXp^;wi5&}0}ipru)G_zq(<@limF z!UCT_8l_~O$5F{y1q;XEPOJjjkOv8_GAPfWh=U6gC`X_cw`j z)U?s5B26EGHW@JGzAULSEg=ryvH6C#TIw|!;E~uf@XeMD~IJD&xiYhIs5#Hsl$MQ-g#s^EHDcFpM-N0(#J0)ll zDKPLz!_Dyu9|ZQ~mQ*(j=o{h(=Zz`6hGs*SpC$z`P*^=42Du}hg&6!Zc!i;mHv1W7 zP9b0ki9)ah{VJk@HDkc4QiQf&rKq3F2tkE5g-Ei+;3|VT1tG+yhzUrfb*_dpQ{X|stayfI9i&~hL*kXvB3Fc@cFoBFkVu0cf735$Hg9CJi`Lj|U z$;3jYh$+Ndk)W^FO6G#1Dxi*FC6=nTT6Eo$uE$$pUtC{QITWT2>y^waZJCONgIfDA zF9G_DG89yRNf_aR@xVPNH5h_zG6-*CIoNET|8gC8@8_Wo7X(HnX?1{sz*@js!ag7g z3M>h%qiDi0lg4@zIA-KfJ<+xb$cQy;TSXKz%@EGWoYfzjq# zjfD&?m}^V3bfB9W;;SB14abdw1#~MD51XqUi|T86!^vjCB!G27{RG+=BR5{TUxJ?v zEI!U^Oh?QQc?`5yjM!zL*BP?#DlP$20XEReA|&jV8f%dW2J5hRR8>QotOL`K_`IW@ z#DP47G~EXI3Pn|Shgo~Uu^93s#>kh3IR|+NBTQOSJwt7WfEjqRBi;j+VQ@hJJ5xv1>i#DT*$!(1W6I=@mkU|mF3z;b4i5Q4x!-pXvmrV9Y6ic$s z2(7BeVzALfJz5jESfrHC`4!T`#C-A$;lyrJ)s;S=@Wz2HCMEA!`Wvjz# z)yJF7P?gP?#OaB%yhcK_t*&%`BnKv%_#Y^9C~mC?KH>#p4W%6IRhh zIu&y~!m&|f%2anysvjj|-kPts8O*qxgx-`-6`Pq-zV2700AC1%xke#mj#qp8=?1hx z8S-+5@EA}N&)JPWwq-@+ykB9AwrnC<1YW|Xn@%R*8_mc`BIk7)>RvuBF;+|>ELKm` zquI+^6j-pkS_Bh_vNRZ2#%HU`dXAE62albh#0J$uV;xyx7;?yab-L>b6DzR@DS1Pf z3p~I)gR|NiXTIdh%p#5MlW!DP)AhYE(qub8Y`Ea zv3fdYFCe1Xm@Ux>p<>{D`Jks63l{U~vYU3Ww9zsazR%k z?(R2A1w5z7wMZVzI|6BEe^yG1U{6?;P)B=uyb2r|)3*-x_CmIb&zcDnemdVD^5m1& z-g>H5H&!aS{uV}!@{+*YW33D!sXh%uhrKnf!fd5xJ zUYyX>FbX0n1*V0X+Dt!aCt*Kp4BnJH{b(^G$J$L6I0Th6ZW%R#d}m1D zojiK5o}fk%bezQ*jA#0zHmYoimEzV^+9b4i)Sy(#ikFWvac{DQn;LO%1CO+%e9c#` zq&f zNIQZ>&}JW}#M`{Mg&Kg@Y40E^hQ%4u-0)=4;t)>J0-%hOmPT_l;GzZEiGiHON^M(i zR0@f_QUhE6|74HD9Liym2vNnXKfux$=)&MVgJG%ZLZZOi~s;H?^eL?hAO|Xr!KxIl0 zfow;HhN7zwDbdQ1L=%(Y-XI!<98C%4sIdX}iWZ@T2uG|g%2+-L{ArLwN}AsRJRf{C z6(THl;4y>zP4jVFLKdPNTBIfiQKKesvra@7A|eSRa3B+0P4if8fSR3>PzZ$V2DSz< zfN(D?VZ5Se?D{%`b|OqfhZ_K^pj{Gy28JWgMqiG=G=KF=xPhYx;0DR!G+9cf#ZWk8 zMLgMjn5uhIfn>uYdYXQH(;=sEbJ$LKgqoSC`%oDxO1){kDkh^zwm;=-V5W+~hbUTM zd2?70DZ5Y!HByGUNAzYX98F4K?_GS`?{%5~_v*V!kHIM|dK0B%G#cg!2-!ds5tpP? zPNjzBNsANY2;iP${eKP>hu0pW{y57whz9WGEcjhn5^y65z8GT&XVLyK^*FZS;?M** z{1hI>sSxT^P?ZIa#G^%4i&_nHI-J4nqx*DY1Sw9_L?F$x+5*@8HThB@u5 z25?Rq1Lo<%B)&{Q%mZu3Nyfeh!tNQtDUGKFX@-pAG)s{B{tDXbV&j2aCZEd1!iiYO zZ8TT)7B^v*BrET5WN>#2sEA5uP%owc`7S4_Y&fHognr6jgXml%%c2l52n)%UL2&e^ z?J=`8FdPk<+W8H}f=EbeRh3j8l3ODQp1~vxlkNT z5J8Ttva~AaQ+<_Gp(0o1`fyZf>auFN*3K?{O_N&6Z!pi+2U)qnMB7lpX*jVpl3LT! zgG+Pm6y8$EV~xq&jp+ljZ6Y9cp)W3qsY<0!Di>>|;h9cdHk`|XX%6Q8M)h*as65(8 z7)@J~f4(r)R6PG*o5)B9TAXfl_L6LrdH^ehKb%oV+( zBzPI5h*MIkyuxI)EYlA?4C)Zi)5P!?2Zbmu+8m-SZtP1s9F{(FQcU+H>{g`DVUJrK zCYuRy*bE1ED|Axp!%Yo!0~LpEdxm6WnZBL>Ua@&#<`0m82|O||e);C{-CfEVZ=d*X5wDBW(12SVcq2DzPGW>~l!hbwJVbbhwK4h4U96AXedS>^u zQ}o)JEH#{f9H$jdo4ue@IFbtnAdck6`ssmek%n>t; zh{0&E8XP9O#Tq+ehDm$W!~uRWq-G!xto`Nm(>h;0gvT|wQHQ(EriRbOAZS-{AwdLquRikn zUoD&VyZb-eyE;7Uh)t*WUVP#s`@6??jqAF3&4Z?&v_KZN@>(V5A6{#^~JT5A7v09F^(+Nx>-2kMC5qn*v*SJFI>A|@Kdp@jQUcm zy%dU&1zP=?6Xs5sGjH8@FL$quH^Lu({~MOWE^|zI=k6LZQ+qv2o5=*wl1a!RlX}|c zV8rC4?yd>lpb}#>1Z1LXT`Lh1XPZs}Z&hfvvtp_{%s-Heq zTd>Fb?BVa7mpkK-RR>m*)fsa*-U>#lj_Fy+B@)4uh~6?=(BrycF{MWnYk? z1BC;ZsQ8mE@ss`R*7B<6+ro>FK62qh_E_VtLtVi~W?0s3*!_#@b(z1?XK%P;- zO<^y;cip^8cYoqh{yO>g#Yp$>k6iKhz|>dIN4NaPCoeyn-gsQ@(3ft0^ zn_AOvq1LT@ZLZDq+*7ld7v{(>KQpiV*E47CIRC8c_Z>g}Vg3r{%o!ggEyo`2IXt>x z>xTV*+qmh)z{5{O0@4*z?$hmm?v5q8V(Ed?yO8w{;xEp+VfkNr>67Mf|NMpfADX#y z)zPm%xBUK7@BDby%yCyuyJykdrPUqFX1tl0d;87(S5YDUi680Ft$#o8PU-pgZ}yVs zE%fFB*Pxd^RZUDgV{|f zI*>8Y)dCQ$e@WG5X{y%-k%`E7(7;iu`jzF%0)j0PN)FC8vxgN+#1BWs=Mr)MpA zfZ4c8-0oR@MBui|cI_C%@R>W=srUZ|+7!rgtk4r$_CybwdJQEjn{*W%B z8z$>;oKbtwa`?KCEEVASzkvhoJK@a86aW;{B{OZiublMmz``G$_{#W|!R*7AK6%Q= zM-O~55L!2T@;iw)fBN!Urg4jwp8Pa^(<|7o7Mwd}(jMWY4cl(U?l`Kj{DoQ9{O&}{ z$={!S@q3T&?>gz26`9R{pxyY^-yV0=g5OMi_^j+-H*Hv(S9!p`JHyy!V8Y_x{G%`{&ztUB2`7><0VKUtIg-Cz$6i+n*Q&v~D{x z&}~Mh4+B^?GM4{*CDkYm!3pr6B$7b-hMtk`wTp&G3@CJE6pewTNLjKhO{U6-VYqtN z+T;HzUFv)U)^VV#&3-gMoC6Psz%_jgAbx%@$W8yh33AqjYH;ba8p+Hr;Dh_ut+FotuIcn=9Y5blS&oy}ZJu$f{jlEF{J?vQ zJs)g&{Bd1x@4L@MPXBD?mb=e>_u2!QJ5T)Z<#(3dz5e*iPPt~q??2Q{dwcfBTP$z) z+;IK+Rr?-=6bg7`*HC6g^&L#mDicBIcnlXeFMU`OW*i`GraVgtxw(c!s;6= zi|6 zYwr6T8;{{Gf96kpKl}aq_4&tb+&hiuZol|~wV|EA_9QPni7@`~+Yh~P_TIuVQzoAN z%?qbrt9wi(Py7RV^V8C|w%zvYi4Xi}^TQ{8{}r`lSe9<4M!vMM;*Gqag_f7J}D9$sXEI*A~@1B zX-enh{`kotena=&-QD9>gC!ez?m7dR+&RiWd2IPfqYWN(0cbJq%)8IozMq@(?A{9& z9)CgjjBVeTbn98_lM5f4_mjWHZfAWGzZ(7foxjX(T)yr+*Mj5!vi*vE^Vfam`uyCf zhbZR}{f(pVKJDI^YuN*LZPPa|;Ah;la+c?_-shvA&zt|9!=CwZ + + keychain-access-groups + + X98UFR7HA3.com.kolide.agent + + + diff --git a/ee/secureenclavesigner/test_app_resources/info.plist b/ee/secureenclavesigner/test_app_resources/info.plist new file mode 100644 index 000000000..fe801acec --- /dev/null +++ b/ee/secureenclavesigner/test_app_resources/info.plist @@ -0,0 +1,20 @@ + + + + + CFBundleExecutable + launcher_test + CFBundleIdentifier + com.kolide.agent + CFBundleName + launcher_test + LSUIElement + + CFBundlePackageType + APPL + CFBundleShortVersionString + 0.1 + CFBundleVersion + 0.1 + + diff --git a/ee/secureenclavesigner/test_app_resources/readme.md b/ee/secureenclavesigner/test_app_resources/readme.md new file mode 100644 index 000000000..bd2d3578f --- /dev/null +++ b/ee/secureenclavesigner/test_app_resources/readme.md @@ -0,0 +1,25 @@ +# Running Tests + +The files in this directory are used only for testing. + +The secure enclave keyer requires apple entitlements in order to be able to access the secure enclave to generate keys and perform cryptographic operations. In order to do this we build the secure enclave go tests to a binary, sign that binary with the required MacOS entitlements, then execute the binary and inspect the output. This is all done via the `TestSecureEnclaveTestRunner` function. + +In order to add entitlements we first need to create a MacOS app with the following structure: + +```sh +launcher_test.app + └── Contents + ├── Info.plist + ├── MacOS + │ └── launcher_test # <- this is the go test binary mentioned above + └── embedded.provisionprofile +``` + +Then we pass the top level directory to the MacOS codsign utility. + +In order to succesfully sign the app with entitlements, there are a few steps that must be completed on the machine in order to run the tests. + +1. Download and install a certificate from the Apple Developer account of type "Mac Development" https://developer.apple.com/account/resources/certificates/list +2. Add you device to the developer account using the "Provisioning UDID" found at Desktop Menu Applie Icon> About This Mac > More Info > System Report https://developer.apple.com/account/resources/devices/list +3. Create a provisioing profile that includes the device https://developer.apple.com/account/resources/profiles/list ... should probably include all devices on the team and be updated in the repo +4. Replace the `embedded.provisionprofile` file with the new profile diff --git a/pkg/osquery/table/launcher_info.go b/pkg/osquery/table/launcher_info.go index 4369e44a3..efbfb4b17 100644 --- a/pkg/osquery/table/launcher_info.go +++ b/pkg/osquery/table/launcher_info.go @@ -5,6 +5,8 @@ import ( "context" "crypto/x509" "encoding/base64" + "encoding/json" + "fmt" "runtime" "github.com/kolide/kit/version" @@ -98,6 +100,17 @@ func generateLauncherInfoTable(store types.GetterSetter) table.GenerateFunc { return results, nil } + if runtime.GOOS == "darwin" && agent.HardwareKeys() != nil && agent.HardwareKeys().Public() != nil { + jsonBytes, err := json.Marshal(agent.HardwareKeys()) + if err != nil { + return nil, fmt.Errorf("marshalling hardware keys: %w", err) + } + results[0]["hardware_key"] = string(jsonBytes) + results[0]["hardware_key_source"] = agent.HardwareKeys().Type() + + return results, nil + } + if hardwareKeyDer, err := x509.MarshalPKIXPublicKey(agent.HardwareKeys().Public()); err == nil { // der is a binary format, so convert to b64 results[0]["hardware_key"] = base64.StdEncoding.EncodeToString(hardwareKeyDer) From d41f914a799944982a6d28335882e731a8e34f41 Mon Sep 17 00:00:00 2001 From: james pickett Date: Thu, 7 Mar 2024 10:54:38 -0800 Subject: [PATCH 02/13] update root dir for secure enclave signer test --- ee/secureenclavesigner/secureenclavesigner_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/secureenclavesigner/secureenclavesigner_test.go b/ee/secureenclavesigner/secureenclavesigner_test.go index 88d32dce4..487c89762 100644 --- a/ee/secureenclavesigner/secureenclavesigner_test.go +++ b/ee/secureenclavesigner/secureenclavesigner_test.go @@ -40,8 +40,10 @@ func TestSecureEnclaveSigner(t *testing.T) { t.Skipf("\nskipping because %s env var was not empty, this is being run in a CI environment without access to secure enclave", testWrappedEnvVarKey) } - // set up app bundle - rootDir := "/tmp/secure_enclave_test" + // put the root dir somewhere else if you want to persist the signed macos app bundle + // rootDir := "/tmp/secure_enclave_test" + + rootDir := t.TempDir() appRoot := filepath.Join(rootDir, "launcher_test.app") // make required dirs krypto_test.app/Contents/MacOS and add files From b1cf53358be358564c47e523c1373056c9e766b7 Mon Sep 17 00:00:00 2001 From: james pickett Date: Thu, 7 Mar 2024 13:00:27 -0800 Subject: [PATCH 03/13] updates secure enclave signer to fetch own data from store --- cmd/launcher/secure_enclave_darwin.go | 2 +- ee/agent/keys_darwin.go | 13 +---- .../secureenclavesigner_darwin.go | 55 ++++++++++++------- .../secureenclavesigner_test.go | 33 ++++------- go.mod | 2 +- go.sum | 4 +- 6 files changed, 50 insertions(+), 59 deletions(-) diff --git a/cmd/launcher/secure_enclave_darwin.go b/cmd/launcher/secure_enclave_darwin.go index 4475ffe57..90ce67dd1 100644 --- a/cmd/launcher/secure_enclave_darwin.go +++ b/cmd/launcher/secure_enclave_darwin.go @@ -13,7 +13,7 @@ import ( "github.com/kolide/launcher/ee/secureenclavesigner" ) -// runSecureEnclave performs either a create-key or sign operation using the secure enclave. +// 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. diff --git a/ee/agent/keys_darwin.go b/ee/agent/keys_darwin.go index 8df4db9ba..4c120bdd0 100644 --- a/ee/agent/keys_darwin.go +++ b/ee/agent/keys_darwin.go @@ -14,15 +14,6 @@ import ( "github.com/kolide/launcher/ee/secureenclavesigner" ) -// persister satisfies the persister interface for secureenclavesigner -type persister struct { - store types.GetterSetterDeleter -} - -func (p *persister) Persist(data []byte) error { - return storeKeyData(p.store, nil, data) -} - func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { // fetch any existing key data @@ -31,7 +22,7 @@ func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (k return nil, err } - ses, err := secureenclavesigner.New(slogger, &persister{store: store}) + ses, err := secureenclavesigner.New(slogger, store) if err != nil { return nil, fmt.Errorf("creating secureenclave signer: %w", err) } @@ -45,7 +36,7 @@ func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (k ) clearKeyData(slogger, store) - ses, err = secureenclavesigner.New(slogger, &persister{store: store}) + ses, err = secureenclavesigner.New(slogger, store) if err != nil { return nil, fmt.Errorf("creating secureenclave signer: %w", err) } diff --git a/ee/secureenclavesigner/secureenclavesigner_darwin.go b/ee/secureenclavesigner/secureenclavesigner_darwin.go index c6a7940ff..0034bd721 100644 --- a/ee/secureenclavesigner/secureenclavesigner_darwin.go +++ b/ee/secureenclavesigner/secureenclavesigner_darwin.go @@ -17,39 +17,52 @@ import ( "sync" "github.com/kolide/krypto/pkg/echelper" + "github.com/kolide/launcher/ee/agent/types" "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/consoleuser" ) const ( - CreateKeyCmd = "create-key" + CreateKeyCmd = "create-key" + PublicEccDataKey = "publicEccData" ) -type opt func(*SecureEnclaveSigner) +type opt func(*secureEnclaveSigner) -type SecureEnclaveSigner struct { +type secureEnclaveSigner struct { uidPubKeyMap map[string]*ecdsa.PublicKey pathToLauncherBinary string - persister persister + store types.GetterSetterDeleter slogger *slog.Logger mux *sync.Mutex } -// persister is an interface that allows the secure enclave signer to persist -// keys it creates, this is needed since we may need to create a new key anytime -// a new user logs in on macos -type persister interface { - Persist([]byte) error -} - -func New(slogger *slog.Logger, persister persister, opts ...opt) (*SecureEnclaveSigner, error) { - ses := &SecureEnclaveSigner{ +func New(slogger *slog.Logger, store types.GetterSetterDeleter, opts ...opt) (*secureEnclaveSigner, error) { + ses := &secureEnclaveSigner{ uidPubKeyMap: make(map[string]*ecdsa.PublicKey), - persister: persister, + store: store, slogger: slogger.With("component", "secureenclavesigner"), mux: &sync.Mutex{}, } + data, err := store.Get([]byte(PublicEccDataKey)) + if err != nil { + return nil, fmt.Errorf("getting public ecc data: %w", err) + } + + if data != nil { + if err := json.Unmarshal(data, ses); err != nil { + ses.slogger.Log(context.TODO(), slog.LevelError, + "unable to unmarshal secure enclave signer, data may be corrupt, wiping", + "err", err, + ) + + if err := store.Delete([]byte(PublicEccDataKey)); err != nil { + return nil, fmt.Errorf("deleting public ecc data: %w", err) + } + } + } + for _, opt := range opts { opt(ses) } @@ -68,7 +81,7 @@ func New(slogger *slog.Logger, persister persister, opts ...opt) (*SecureEnclave // Public returns the public key of the current console user // creating and peristing a new one if needed -func (ses *SecureEnclaveSigner) Public() crypto.PublicKey { +func (ses *secureEnclaveSigner) Public() crypto.PublicKey { ses.mux.Lock() defer ses.mux.Unlock() @@ -110,7 +123,7 @@ func (ses *SecureEnclaveSigner) Public() crypto.PublicKey { return nil } - if err := ses.persister.Persist(json); err != nil { + if err := ses.store.Set([]byte(PublicEccDataKey), json); err != nil { ses.slogger.Log(context.TODO(), slog.LevelError, "persisting secure enclave signer", "err", err, @@ -122,11 +135,11 @@ func (ses *SecureEnclaveSigner) Public() crypto.PublicKey { return key } -func (ses *SecureEnclaveSigner) Type() string { +func (ses *secureEnclaveSigner) Type() string { return "secure_enclave" } -func (ses *SecureEnclaveSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { +func (ses *secureEnclaveSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { return nil, fmt.Errorf("not implemented") } @@ -135,7 +148,7 @@ type keyData struct { PubKey string `json:"pub_key"` } -func (ses *SecureEnclaveSigner) MarshalJSON() ([]byte, error) { +func (ses *secureEnclaveSigner) MarshalJSON() ([]byte, error) { var keyDatas []keyData for uid, pubKey := range ses.uidPubKeyMap { @@ -154,7 +167,7 @@ func (ses *SecureEnclaveSigner) MarshalJSON() ([]byte, error) { return json.Marshal(keyDatas) } -func (ses *SecureEnclaveSigner) UnmarshalJSON(data []byte) error { +func (ses *secureEnclaveSigner) UnmarshalJSON(data []byte) error { if ses.uidPubKeyMap == nil { ses.uidPubKeyMap = make(map[string]*ecdsa.PublicKey) } @@ -176,7 +189,7 @@ func (ses *SecureEnclaveSigner) UnmarshalJSON(data []byte) error { return nil } -func (ses *SecureEnclaveSigner) createKey(ctx context.Context, u *user.User) (*ecdsa.PublicKey, error) { +func (ses *secureEnclaveSigner) createKey(ctx context.Context, u *user.User) (*ecdsa.PublicKey, error) { cmd, err := allowedcmd.Launchctl( ctx, "asuser", diff --git a/ee/secureenclavesigner/secureenclavesigner_test.go b/ee/secureenclavesigner/secureenclavesigner_test.go index 487c89762..b26000646 100644 --- a/ee/secureenclavesigner/secureenclavesigner_test.go +++ b/ee/secureenclavesigner/secureenclavesigner_test.go @@ -6,7 +6,6 @@ package secureenclavesigner import ( "context" "crypto/ecdsa" - "encoding/json" "fmt" "os" "os/exec" @@ -15,9 +14,8 @@ import ( "time" "github.com/kolide/krypto/pkg/echelper" - "github.com/kolide/launcher/ee/secureenclavesigner/mocks" + "github.com/kolide/launcher/ee/agent/storage/inmemory" "github.com/kolide/launcher/pkg/log/multislogger" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -27,7 +25,7 @@ const ( ) func WithBinaryPath(p string) opt { - return func(ses *SecureEnclaveSigner) { + return func(ses *secureEnclaveSigner) { ses.pathToLauncherBinary = p } } @@ -81,15 +79,14 @@ func TestSecureEnclaveSigner(t *testing.T) { // sign app bundle signApp(t, appRoot) - persister := mocks.NewPersister(t) + store := inmemory.NewStore() // create brand new signer without existing key // ask for public first to trigger key generation - ses, err := New(multislogger.NewNopLogger(), persister, WithBinaryPath(executablePath)) - require.NoError(t, err) - - // should only call persist once on first public key generation - persister.On("Persist", mock.Anything).Return(nil).Once() + ses, err := New(multislogger.NewNopLogger(), store, WithBinaryPath(executablePath)) + require.NoError(t, err, + "should be able to create secure enclave signer", + ) pubKey := ses.Public() require.NotNil(t, pubKey, @@ -115,22 +112,12 @@ func TestSecureEnclaveSigner(t *testing.T) { "asking for the same public key should return the same key", ) - jsonBytes, err := json.Marshal(ses) + existingDataSes, err := New(multislogger.NewNopLogger(), store, WithBinaryPath(executablePath)) require.NoError(t, err, - "should be able to marshal secure enclave signer to json", + "should be able to create secure enclave signer with existing key", ) - unmarshalledSes, err := New(multislogger.NewNopLogger(), persister, WithBinaryPath(executablePath)) - require.NoError(t, err) - - require.NoError(t, json.Unmarshal(jsonBytes, &unmarshalledSes), - "should be able to unmarshal secure enclave signer from json", - ) - - // use same test binary - unmarshalledSes.pathToLauncherBinary = ses.pathToLauncherBinary - - pubKeyUnmarshalled := unmarshalledSes.Public() + pubKeyUnmarshalled := existingDataSes.Public() require.NotNil(t, pubKeyUnmarshalled, "should be able to get public key from unmarshalled secure enclave signer", ) diff --git a/go.mod b/go.mod index da9ab44fc..247fac442 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/groob/plist v0.0.0-20190114192801-a99fbe489d03 github.com/knightsc/system_policy v1.1.1-0.20211029142728-5f4c0d5419cc github.com/kolide/kit v0.0.0-20221107170827-fb85e3d59eab - github.com/kolide/krypto v0.1.1-0.20231219012048-5859599c50aa + github.com/kolide/krypto v0.1.1-0.20231229162826-db516b7e0121 github.com/mat/besticon v3.9.0+incompatible github.com/mattn/go-sqlite3 v1.14.19 github.com/mixer/clock v0.0.0-20170901150240-b08e6b4da7ea diff --git a/go.sum b/go.sum index c9bd5a693..0df375891 100644 --- a/go.sum +++ b/go.sum @@ -167,8 +167,8 @@ github.com/knightsc/system_policy v1.1.1-0.20211029142728-5f4c0d5419cc h1:g2S0GQ github.com/knightsc/system_policy v1.1.1-0.20211029142728-5f4c0d5419cc/go.mod h1:5e34JEkxWsOeAd9jvcxkz01tAY/JAGFuabGnNBJ6TT4= github.com/kolide/kit v0.0.0-20221107170827-fb85e3d59eab h1:KVR7cs+oPyy85i+8t1ZaNSy1bymCy5FuWyt51pdrXu4= github.com/kolide/kit v0.0.0-20221107170827-fb85e3d59eab/go.mod h1:OYYulo9tUqRadRLwB0+LE914sa1ui2yL7OrcU3Q/1XY= -github.com/kolide/krypto v0.1.1-0.20231219012048-5859599c50aa h1:3agrIh6HWiEZAvH3ubVpfsaRFsg1Ux+1S5HU+HE5pPI= -github.com/kolide/krypto v0.1.1-0.20231219012048-5859599c50aa/go.mod h1:/0sxd3OIxciTlMTeZI/9WTaUHsx/K/+3f+NbD5dywTY= +github.com/kolide/krypto v0.1.1-0.20231229162826-db516b7e0121 h1:f7APX9VNsCkD/tdlAjbU4A22FyfTOCF6QadlvnzZElg= +github.com/kolide/krypto v0.1.1-0.20231229162826-db516b7e0121/go.mod h1:/0sxd3OIxciTlMTeZI/9WTaUHsx/K/+3f+NbD5dywTY= github.com/kolide/systray v1.10.4 h1:eBhnVfhW0fGal1KBkBZC9fzRs4yrxUymgiXuQh5MBSg= github.com/kolide/systray v1.10.4/go.mod h1:FwK9yUmU3JO+vA7TOLQSFRgEQ3euLxOqic5qlBtFrik= github.com/kolide/toast v1.0.2 h1:BQlIfO3wbKIEWfF0c8v4UkdhSIZYnSWaKkZl+Yarptk= From 78a0cd433d353e268a38f50f5363712d82cac38e Mon Sep 17 00:00:00 2001 From: james pickett Date: Thu, 7 Mar 2024 13:10:27 -0800 Subject: [PATCH 04/13] remove unneeded code --- ee/agent/keys_darwin.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/ee/agent/keys_darwin.go b/ee/agent/keys_darwin.go index 4c120bdd0..e36a10f7b 100644 --- a/ee/agent/keys_darwin.go +++ b/ee/agent/keys_darwin.go @@ -4,8 +4,6 @@ package agent import ( - "context" - "encoding/json" "errors" "fmt" "log/slog" @@ -15,34 +13,11 @@ import ( ) func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { - - // fetch any existing key data - _, pubData, err := fetchKeyData(store) - if err != nil { - return nil, err - } - ses, err := secureenclavesigner.New(slogger, store) if err != nil { return nil, fmt.Errorf("creating secureenclave signer: %w", err) } - if pubData != nil { - if err := json.Unmarshal(pubData, &ses); err != nil { - // data is corrupt or not in the expected format, clear it - slogger.Log(context.TODO(), slog.LevelError, - "could not unmarshal stored key data, clearing key data and generating new keys", - "err", err, - ) - clearKeyData(slogger, store) - - ses, err = secureenclavesigner.New(slogger, store) - if err != nil { - return nil, fmt.Errorf("creating secureenclave signer: %w", err) - } - } - } - // this is kind of weird, but we need to call public to ensure the key is generated // it's done this way to do satisfying signer interface which doesn't return an error if ses.Public() == nil { From 4ee293d848dbf94f9e9c8183cdc4f56b226f54ac Mon Sep 17 00:00:00 2001 From: james pickett Date: Thu, 7 Mar 2024 13:41:00 -0800 Subject: [PATCH 05/13] remove unneeded mock --- ee/secureenclavesigner/mocks/persister.go | 38 ------------------- .../secureenclavesigner_test.go | 1 + 2 files changed, 1 insertion(+), 38 deletions(-) delete mode 100644 ee/secureenclavesigner/mocks/persister.go diff --git a/ee/secureenclavesigner/mocks/persister.go b/ee/secureenclavesigner/mocks/persister.go deleted file mode 100644 index 7a96082c9..000000000 --- a/ee/secureenclavesigner/mocks/persister.go +++ /dev/null @@ -1,38 +0,0 @@ -// Code generated by mockery v2.33.3. DO NOT EDIT. - -package mocks - -import mock "github.com/stretchr/testify/mock" - -// Persister is an autogenerated mock type for the persister type -type Persister struct { - mock.Mock -} - -// Persist provides a mock function with given fields: _a0 -func (_m *Persister) Persist(_a0 []byte) error { - ret := _m.Called(_a0) - - var r0 error - if rf, ok := ret.Get(0).(func([]byte) error); ok { - r0 = rf(_a0) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// NewPersister creates a new instance of Persister. 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 NewPersister(t interface { - mock.TestingT - Cleanup(func()) -}) *Persister { - mock := &Persister{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/ee/secureenclavesigner/secureenclavesigner_test.go b/ee/secureenclavesigner/secureenclavesigner_test.go index b26000646..b92986ad3 100644 --- a/ee/secureenclavesigner/secureenclavesigner_test.go +++ b/ee/secureenclavesigner/secureenclavesigner_test.go @@ -39,6 +39,7 @@ func TestSecureEnclaveSigner(t *testing.T) { } // put the root dir somewhere else if you want to persist the signed macos app bundle + // should build this into make at some point // rootDir := "/tmp/secure_enclave_test" rootDir := t.TempDir() From f1ebb6f5806b2828d70eddce3a419acb7cdcbbc1 Mon Sep 17 00:00:00 2001 From: james pickett Date: Fri, 8 Mar 2024 09:41:46 -0800 Subject: [PATCH 06/13] add tracing, create key in new --- ee/agent/keys.go | 8 +- ee/agent/keys_darwin.go | 17 +-- ee/agent/keys_tpm.go | 14 +- ee/debug/shipper/shipper_test.go | 3 +- .../secureenclavesigner_darwin.go | 142 ++++++++++++------ .../secureenclavesigner_test.go | 8 +- pkg/osquery/extension.go | 2 +- 7 files changed, 125 insertions(+), 69 deletions(-) diff --git a/ee/agent/keys.go b/ee/agent/keys.go index 22bddb340..920f83720 100644 --- a/ee/agent/keys.go +++ b/ee/agent/keys.go @@ -10,6 +10,7 @@ import ( "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 { @@ -28,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) error { + ctx, span := traces.StartSpan(ctx) + defer span.End() + slogger = slogger.With("component", "agentkeys") var err error @@ -40,7 +44,7 @@ func SetupKeys(slogger *slog.Logger, store types.GetterSetterDeleter) error { } err = backoff.WaitFor(func() error { - hwKeys, err := setupHardwareKeys(slogger, store) + hwKeys, err := setupHardwareKeys(ctx, slogger, store) if err != nil { return err } diff --git a/ee/agent/keys_darwin.go b/ee/agent/keys_darwin.go index e36a10f7b..891969f21 100644 --- a/ee/agent/keys_darwin.go +++ b/ee/agent/keys_darwin.go @@ -4,25 +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" ) -func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { - ses, err := secureenclavesigner.New(slogger, store) +func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { + ctx, span := traces.StartSpan(ctx) + defer span.End() + + 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) } - // this is kind of weird, but we need to call public to ensure the key is generated - // it's done this way to do satisfying signer interface which doesn't return an error - if ses.Public() == nil { - return nil, errors.New("public key was not be created") - } - return ses, nil } diff --git a/ee/agent/keys_tpm.go b/ee/agent/keys_tpm.go index 330cb5a8f..ded82316b 100644 --- a/ee/agent/keys_tpm.go +++ b/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) { + ctx, 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) } diff --git a/ee/debug/shipper/shipper_test.go b/ee/debug/shipper/shipper_test.go index 49ea7e7b7..2e15fc57c 100644 --- a/ee/debug/shipper/shipper_test.go +++ b/ee/debug/shipper/shipper_test.go @@ -1,6 +1,7 @@ package shipper import ( + "context" "encoding/json" "fmt" "io" @@ -56,7 +57,7 @@ func TestShip(t *testing.T) { //nolint:paralleltest name: "happy path with signing keys and enroll secret", mockKnapsack: func(t *testing.T) *typesMocks.Knapsack { configStore := inmemory.NewStore() - agent.SetupKeys(multislogger.NewNopLogger(), configStore) + agent.SetupKeys(context.TODO(), multislogger.NewNopLogger(), configStore) k := typesMocks.NewKnapsack(t) k.On("EnrollSecret").Return("enroll_secret_value") diff --git a/ee/secureenclavesigner/secureenclavesigner_darwin.go b/ee/secureenclavesigner/secureenclavesigner_darwin.go index 0034bd721..7dcc1492c 100644 --- a/ee/secureenclavesigner/secureenclavesigner_darwin.go +++ b/ee/secureenclavesigner/secureenclavesigner_darwin.go @@ -8,6 +8,7 @@ import ( "crypto" "crypto/ecdsa" "encoding/json" + "errors" "fmt" "io" "log/slog" @@ -20,6 +21,7 @@ import ( "github.com/kolide/launcher/ee/agent/types" "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/ee/consoleuser" + "github.com/kolide/launcher/pkg/traces" ) const ( @@ -37,7 +39,10 @@ type secureEnclaveSigner struct { mux *sync.Mutex } -func New(slogger *slog.Logger, store types.GetterSetterDeleter, opts ...opt) (*secureEnclaveSigner, error) { +func New(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter, opts ...opt) (*secureEnclaveSigner, error) { + ctx, span := traces.StartSpan(ctx) + defer span.End() + ses := &secureEnclaveSigner{ uidPubKeyMap: make(map[string]*ecdsa.PublicKey), store: store, @@ -47,18 +52,21 @@ func New(slogger *slog.Logger, store types.GetterSetterDeleter, opts ...opt) (*s data, err := store.Get([]byte(PublicEccDataKey)) if err != nil { - return nil, fmt.Errorf("getting public ecc data: %w", err) + traces.SetError(span, fmt.Errorf("getting public ecc data from store: %w", err)) + return nil, fmt.Errorf("getting public ecc data from store: %w", err) } if data != nil { if err := json.Unmarshal(data, ses); err != nil { + traces.SetError(span, fmt.Errorf("unmarshaling secure enclave signer: %w", err)) ses.slogger.Log(context.TODO(), slog.LevelError, "unable to unmarshal secure enclave signer, data may be corrupt, wiping", "err", err, ) if err := store.Delete([]byte(PublicEccDataKey)); err != nil { - return nil, fmt.Errorf("deleting public ecc data: %w", err) + traces.SetError(span, fmt.Errorf("deleting corrupt public ecc data: %w", err)) + return nil, fmt.Errorf("deleting corrupt public ecc data: %w", err) } } } @@ -70,69 +78,41 @@ func New(slogger *slog.Logger, store types.GetterSetterDeleter, opts ...opt) (*s if ses.pathToLauncherBinary == "" { p, err := os.Executable() if err != nil { + traces.SetError(span, fmt.Errorf("getting path to launcher binary: %w", err)) return nil, fmt.Errorf("getting path to launcher binary: %w", err) } ses.pathToLauncherBinary = p } - return ses, nil -} - -// Public returns the public key of the current console user -// creating and peristing a new one if needed -func (ses *secureEnclaveSigner) Public() crypto.PublicKey { - ses.mux.Lock() - defer ses.mux.Unlock() - - c, err := firstConsoleUser() - if err != nil { + // get current console user key to make sure it's available + if _, err := ses.currentConsoleUserKey(ctx); err != nil { + traces.SetError(span, fmt.Errorf("getting current console user key: %w", err)) ses.slogger.Log(context.TODO(), slog.LevelError, - "getting first console user", + "getting current console user key", "err", err, ) - return nil - } - - if v, ok := ses.uidPubKeyMap[c.Uid]; ok { - return v + // intentionally not returning error here, because this runs on start up + // and maybe the console user or secure enclave is not available yet } - key, err := ses.createKey(context.TODO(), c) - if err != nil { - ses.slogger.Log(context.TODO(), slog.LevelError, - "creating key", - "err", err, - ) - - return nil - } - - ses.uidPubKeyMap[c.Uid] = key + return ses, nil +} - // pesist the new key - json, err := json.Marshal(ses) +// Public returns the public key of the current console user +// creating and peristing a new one if needed +func (ses *secureEnclaveSigner) Public() crypto.PublicKey { + k, err := ses.currentConsoleUserKey(context.TODO()) if err != nil { ses.slogger.Log(context.TODO(), slog.LevelError, - "marshaling secure enclave signer", + "getting public key", "err", err, ) - - delete(ses.uidPubKeyMap, c.Uid) return nil } - if err := ses.store.Set([]byte(PublicEccDataKey), json); err != nil { - ses.slogger.Log(context.TODO(), slog.LevelError, - "persisting secure enclave signer", - "err", err, - ) - delete(ses.uidPubKeyMap, c.Uid) - return nil - } - - return key + return k } func (ses *secureEnclaveSigner) Type() string { @@ -148,6 +128,44 @@ type keyData struct { PubKey string `json:"pub_key"` } +func (ses *secureEnclaveSigner) currentConsoleUserKey(ctx context.Context) (*ecdsa.PublicKey, error) { + ctx, span := traces.StartSpan(ctx) + defer span.End() + + ses.mux.Lock() + defer ses.mux.Unlock() + + cu, err := firstConsoleUser(ctx) + if err != nil { + traces.SetError(span, fmt.Errorf("getting first console user: %w", err)) + return nil, fmt.Errorf("getting first console user: %w", err) + } + + key, ok := ses.uidPubKeyMap[cu.Uid] + if ok { + span.AddEvent("found_existing_key_for_console_user") + return key, nil + } + + key, err = ses.createKey(ctx, cu) + if err != nil { + traces.SetError(span, fmt.Errorf("creating key: %w", err)) + return nil, fmt.Errorf("creating key: %w", err) + } + + span.AddEvent("created_new_key_for_console_user") + + ses.uidPubKeyMap[cu.Uid] = key + if err := ses.save(); err != nil { + delete(ses.uidPubKeyMap, cu.Uid) + traces.SetError(span, fmt.Errorf("saving secure enclave signer: %w", err)) + return nil, fmt.Errorf("saving secure enclave signer: %w", err) + } + + span.AddEvent("saved_key_for_console_user") + return key, nil +} + func (ses *secureEnclaveSigner) MarshalJSON() ([]byte, error) { var keyDatas []keyData @@ -190,6 +208,9 @@ func (ses *secureEnclaveSigner) UnmarshalJSON(data []byte) error { } func (ses *secureEnclaveSigner) createKey(ctx context.Context, u *user.User) (*ecdsa.PublicKey, error) { + ctx, span := traces.StartSpan(ctx) + defer span.End() + cmd, err := allowedcmd.Launchctl( ctx, "asuser", @@ -204,6 +225,7 @@ func (ses *secureEnclaveSigner) createKey(ctx context.Context, u *user.User) (*e ) if err != nil { + traces.SetError(span, fmt.Errorf("creating command to create key: %w", err)) return nil, fmt.Errorf("creating command to create key: %w", err) } @@ -211,12 +233,14 @@ func (ses *secureEnclaveSigner) createKey(ctx context.Context, u *user.User) (*e cmd.Env = append(cmd.Environ(), fmt.Sprintf("%s=%s", "LAUNCHER_SKIP_UPDATES", "true")) out, err := cmd.CombinedOutput() if err != nil { + traces.SetError(span, fmt.Errorf("executing launcher binary to create key: %w: %s", err, string(out))) return nil, fmt.Errorf("executing launcher binary to create key: %w: %s", err, string(out)) } pubKey, err := echelper.PublicB64DerToEcdsaKey([]byte(lastLine(out))) if err != nil { - return nil, fmt.Errorf("marshalling public key to der: %w", err) + traces.SetError(span, fmt.Errorf("converting public key to ecdsa: %w", err)) + return nil, fmt.Errorf("converting public key to ecdsa: %w", err) } return pubKey, nil @@ -239,15 +263,33 @@ func lastLine(out []byte) string { return lastLine } -func firstConsoleUser() (*user.User, error) { - c, err := consoleuser.CurrentUsers(context.TODO()) +func firstConsoleUser(ctx context.Context) (*user.User, error) { + ctx, span := traces.StartSpan(ctx) + defer span.End() + + c, err := consoleuser.CurrentUsers(ctx) if err != nil { + traces.SetError(span, fmt.Errorf("getting current users: %w", err)) return nil, fmt.Errorf("getting current users: %w", err) } if len(c) == 0 { - return nil, fmt.Errorf("no console users found") + traces.SetError(span, errors.New("no console users found")) + return nil, errors.New("no console users found") } return c[0], nil } + +func (ses *secureEnclaveSigner) save() error { + json, err := json.Marshal(ses) + if err != nil { + return fmt.Errorf("marshaling secure enclave signer: %w", err) + } + + if err := ses.store.Set([]byte(PublicEccDataKey), json); err != nil { + return fmt.Errorf("setting public ecc data: %w", err) + } + + return nil +} diff --git a/ee/secureenclavesigner/secureenclavesigner_test.go b/ee/secureenclavesigner/secureenclavesigner_test.go index b92986ad3..ab834e84d 100644 --- a/ee/secureenclavesigner/secureenclavesigner_test.go +++ b/ee/secureenclavesigner/secureenclavesigner_test.go @@ -40,9 +40,9 @@ func TestSecureEnclaveSigner(t *testing.T) { // put the root dir somewhere else if you want to persist the signed macos app bundle // should build this into make at some point - // rootDir := "/tmp/secure_enclave_test" + rootDir := "/tmp/secure_enclave_test" - rootDir := t.TempDir() + // rootDir := t.TempDir() appRoot := filepath.Join(rootDir, "launcher_test.app") // make required dirs krypto_test.app/Contents/MacOS and add files @@ -84,7 +84,7 @@ func TestSecureEnclaveSigner(t *testing.T) { // create brand new signer without existing key // ask for public first to trigger key generation - ses, err := New(multislogger.NewNopLogger(), store, WithBinaryPath(executablePath)) + ses, err := New(context.TODO(), multislogger.NewNopLogger(), store, WithBinaryPath(executablePath)) require.NoError(t, err, "should be able to create secure enclave signer", ) @@ -113,7 +113,7 @@ func TestSecureEnclaveSigner(t *testing.T) { "asking for the same public key should return the same key", ) - existingDataSes, err := New(multislogger.NewNopLogger(), store, WithBinaryPath(executablePath)) + existingDataSes, err := New(context.TODO(), multislogger.NewNopLogger(), store, WithBinaryPath(executablePath)) require.NoError(t, err, "should be able to create secure enclave signer with existing key", ) diff --git a/pkg/osquery/extension.go b/pkg/osquery/extension.go index 583b03064..5b2d51500 100644 --- a/pkg/osquery/extension.go +++ b/pkg/osquery/extension.go @@ -125,7 +125,7 @@ func NewExtension(ctx context.Context, client service.KolideService, k types.Kna return nil, fmt.Errorf("setting up initial launcher keys: %w", err) } - if err := agent.SetupKeys(slogger, configStore); err != nil { + if err := agent.SetupKeys(ctx, slogger, configStore); err != nil { return nil, fmt.Errorf("setting up agent keys: %w", err) } From f730fb29403c30d4d780cfac5044923bb33ed46c Mon Sep 17 00:00:00 2001 From: james pickett Date: Fri, 8 Mar 2024 09:50:51 -0800 Subject: [PATCH 07/13] lint --- ee/agent/keys_tpm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/agent/keys_tpm.go b/ee/agent/keys_tpm.go index ded82316b..82d057ca2 100644 --- a/ee/agent/keys_tpm.go +++ b/ee/agent/keys_tpm.go @@ -14,7 +14,7 @@ import ( ) func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { - ctx, span := traces.StartSpan(ctx) + _, span := traces.StartSpan(ctx) defer span.End() priData, pubData, err := fetchKeyData(store) From 2a6e7877030c0e2ec80f9f4f5e3eebb2e7d19e97 Mon Sep 17 00:00:00 2001 From: james pickett Date: Fri, 8 Mar 2024 10:25:56 -0800 Subject: [PATCH 08/13] add bool for setting up hardware keys --- ee/agent/keys.go | 6 +++++- ee/debug/shipper/shipper_test.go | 2 +- pkg/osquery/extension.go | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ee/agent/keys.go b/ee/agent/keys.go index 920f83720..f180eb77c 100644 --- a/ee/agent/keys.go +++ b/ee/agent/keys.go @@ -29,7 +29,7 @@ func LocalDbKeys() keyInt { return localDbKeys } -func SetupKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) error { +func SetupKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter, initHardwareKeys bool) error { ctx, span := traces.StartSpan(ctx) defer span.End() @@ -43,6 +43,10 @@ func SetupKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSett return fmt.Errorf("setting up local db keys: %w", err) } + if !initHardwareKeys { + return nil + } + err = backoff.WaitFor(func() error { hwKeys, err := setupHardwareKeys(ctx, slogger, store) if err != nil { diff --git a/ee/debug/shipper/shipper_test.go b/ee/debug/shipper/shipper_test.go index 2e15fc57c..ec2b42678 100644 --- a/ee/debug/shipper/shipper_test.go +++ b/ee/debug/shipper/shipper_test.go @@ -57,7 +57,7 @@ func TestShip(t *testing.T) { //nolint:paralleltest name: "happy path with signing keys and enroll secret", mockKnapsack: func(t *testing.T) *typesMocks.Knapsack { configStore := inmemory.NewStore() - agent.SetupKeys(context.TODO(), multislogger.NewNopLogger(), configStore) + agent.SetupKeys(context.TODO(), multislogger.NewNopLogger(), configStore, false) k := typesMocks.NewKnapsack(t) k.On("EnrollSecret").Return("enroll_secret_value") diff --git a/pkg/osquery/extension.go b/pkg/osquery/extension.go index 5b2d51500..0bfb83a97 100644 --- a/pkg/osquery/extension.go +++ b/pkg/osquery/extension.go @@ -125,7 +125,7 @@ func NewExtension(ctx context.Context, client service.KolideService, k types.Kna return nil, fmt.Errorf("setting up initial launcher keys: %w", err) } - if err := agent.SetupKeys(ctx, slogger, configStore); err != nil { + if err := agent.SetupKeys(ctx, slogger, configStore, true); err != nil { return nil, fmt.Errorf("setting up agent keys: %w", err) } From 9a0a3e9a813e3d7dcb3cb9c0bb0856aa279c3c4e Mon Sep 17 00:00:00 2001 From: james pickett Date: Fri, 8 Mar 2024 10:51:40 -0800 Subject: [PATCH 09/13] skip hardware key setup in tests --- ee/agent/keys.go | 4 +- ee/debug/shipper/shipper_test.go | 2 +- pkg/osquery/extension.go | 5 +- pkg/osquery/extension_test.go | 108 +++++++++++++++++++++++-------- 4 files changed, 88 insertions(+), 31 deletions(-) diff --git a/ee/agent/keys.go b/ee/agent/keys.go index f180eb77c..b88914b66 100644 --- a/ee/agent/keys.go +++ b/ee/agent/keys.go @@ -29,7 +29,7 @@ func LocalDbKeys() keyInt { return localDbKeys } -func SetupKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter, initHardwareKeys bool) error { +func SetupKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter, skipHardwareKeys bool) error { ctx, span := traces.StartSpan(ctx) defer span.End() @@ -43,7 +43,7 @@ func SetupKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSett return fmt.Errorf("setting up local db keys: %w", err) } - if !initHardwareKeys { + if skipHardwareKeys { return nil } diff --git a/ee/debug/shipper/shipper_test.go b/ee/debug/shipper/shipper_test.go index ec2b42678..58953a90a 100644 --- a/ee/debug/shipper/shipper_test.go +++ b/ee/debug/shipper/shipper_test.go @@ -57,7 +57,7 @@ func TestShip(t *testing.T) { //nolint:paralleltest name: "happy path with signing keys and enroll secret", mockKnapsack: func(t *testing.T) *typesMocks.Knapsack { configStore := inmemory.NewStore() - agent.SetupKeys(context.TODO(), multislogger.NewNopLogger(), configStore, false) + agent.SetupKeys(context.TODO(), multislogger.NewNopLogger(), configStore, true) k := typesMocks.NewKnapsack(t) k.On("EnrollSecret").Return("enroll_secret_value") diff --git a/pkg/osquery/extension.go b/pkg/osquery/extension.go index 0bfb83a97..294340006 100644 --- a/pkg/osquery/extension.go +++ b/pkg/osquery/extension.go @@ -92,6 +92,9 @@ type ExtensionOpts struct { // RunDifferentialQueriesImmediately allows the client to execute a new query the first time it sees it, // bypassing the scheduler. RunDifferentialQueriesImmediately bool + // skipHardwareKeysSetup is a flag to indicate if we should skip setting up hardware keys. + // This is useful for testing environments where we don't have required hardware. + skipHardwareKeysSetup bool } // NewExtension creates a new Extension from the provided service.KolideService @@ -125,7 +128,7 @@ func NewExtension(ctx context.Context, client service.KolideService, k types.Kna return nil, fmt.Errorf("setting up initial launcher keys: %w", err) } - if err := agent.SetupKeys(ctx, slogger, configStore, true); err != nil { + if err := agent.SetupKeys(ctx, slogger, configStore, opts.skipHardwareKeysSetup); err != nil { return nil, fmt.Errorf("setting up agent keys: %w", err) } diff --git a/pkg/osquery/extension_test.go b/pkg/osquery/extension_test.go index a9fa0ed98..b99d50e71 100644 --- a/pkg/osquery/extension_test.go +++ b/pkg/osquery/extension_test.go @@ -70,7 +70,9 @@ func TestNewExtensionEmptyEnrollSecret(t *testing.T) { m.On("ReadEnrollSecret").Maybe().Return("", errors.New("test")) // We should be able to make an extension despite an empty enroll secret - e, err := NewExtension(context.TODO(), &mock.KolideService{}, m, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), &mock.KolideService{}, m, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) assert.Nil(t, err) assert.NotNil(t, e) } @@ -100,7 +102,9 @@ func TestNewExtensionDatabaseError(t *testing.T) { m.On("ConfigStore").Return(agentbbolt.NewStore(multislogger.NewNopLogger(), db, storage.ConfigStore.String())) m.On("Slogger").Return(multislogger.NewNopLogger()).Maybe() - e, err := NewExtension(context.TODO(), &mock.KolideService{}, m, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), &mock.KolideService{}, m, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) assert.NotNil(t, err) assert.Nil(t, e) } @@ -110,7 +114,9 @@ func TestGetHostIdentifier(t *testing.T) { defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), &mock.KolideService{}, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), &mock.KolideService{}, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) ident, err := e.getHostIdentifier() @@ -125,7 +131,9 @@ func TestGetHostIdentifier(t *testing.T) { db, cleanup = makeTempDB(t) defer cleanup() k = makeKnapsack(t, db) - e, err = NewExtension(context.TODO(), &mock.KolideService{}, k, ExtensionOpts{}) + e, err = NewExtension(context.TODO(), &mock.KolideService{}, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) ident, err = e.getHostIdentifier() @@ -140,7 +148,9 @@ func TestGetHostIdentifierCorruptedData(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), &mock.KolideService{}, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), &mock.KolideService{}, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) // Put garbage UUID in DB @@ -169,7 +179,9 @@ func TestExtensionEnrollTransportError(t *testing.T) { defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) key, invalid, err := e.Enroll(context.Background()) @@ -189,7 +201,9 @@ func TestExtensionEnrollSecretInvalid(t *testing.T) { db, cleanup := makeTempDB(t) k := makeKnapsack(t, db) defer cleanup() - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) key, invalid, err := e.Enroll(context.Background()) @@ -218,7 +232,9 @@ func TestExtensionEnroll(t *testing.T) { expectedEnrollSecret := "foo_secret" k.On("ReadEnrollSecret").Maybe().Return(expectedEnrollSecret, nil) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) key, invalid, err := e.Enroll(context.Background()) @@ -237,7 +253,9 @@ func TestExtensionEnroll(t *testing.T) { assert.Equal(t, expectedNodeKey, key) assert.Equal(t, expectedEnrollSecret, gotEnrollSecret) - e, err = NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err = NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) // Still should not re-enroll (because node key stored in DB) key, invalid, err = e.Enroll(context.Background()) @@ -271,7 +289,9 @@ func TestExtensionGenerateConfigsTransportError(t *testing.T) { defer cleanup() k := makeKnapsack(t, db) k.ConfigStore().Set([]byte(nodeKeyKey), []byte("some_node_key")) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) configs, err := e.GenerateConfigs(context.Background()) @@ -292,7 +312,9 @@ func TestExtensionGenerateConfigsCaching(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) configs, err := e.GenerateConfigs(context.Background()) @@ -329,7 +351,9 @@ func TestExtensionGenerateConfigsEnrollmentInvalid(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) e.NodeKey = "bad_node_key" @@ -356,7 +380,9 @@ func TestGenerateConfigs_CannotEnrollYet(t *testing.T) { k.On("Slogger").Return(multislogger.NewNopLogger()) k.On("ReadEnrollSecret").Maybe().Return("", errors.New("test")) - e, err := NewExtension(context.TODO(), s, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), s, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) configs, err := e.GenerateConfigs(context.Background()) @@ -385,7 +411,9 @@ func TestExtensionGenerateConfigs(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) configs, err := e.GenerateConfigs(context.Background()) @@ -404,7 +432,9 @@ func TestExtensionWriteLogsTransportError(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) err = e.writeLogsWithReenroll(context.Background(), logger.LogTypeSnapshot, []string{"foobar"}, true) @@ -428,7 +458,9 @@ func TestExtensionWriteLogsEnrollmentInvalid(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) e.NodeKey = "bad_node_key" @@ -457,7 +489,9 @@ func TestExtensionWriteLogs(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) e.NodeKey = expectedNodeKey @@ -533,7 +567,9 @@ func TestExtensionWriteBufferedLogsEmpty(t *testing.T) { k.On("Slogger").Return(multislogger.NewNopLogger()).Maybe() k.On("ReadEnrollSecret").Maybe().Return("enroll_secret", nil) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) // No buffered logs should result in success and no remote action being @@ -572,7 +608,9 @@ func TestExtensionWriteBufferedLogs(t *testing.T) { k.On("Slogger").Return(multislogger.NewNopLogger()).Maybe() k.On("ReadEnrollSecret").Maybe().Return("enroll_secret", nil) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) e.LogString(context.Background(), logger.LogTypeStatus, "status foo") @@ -642,7 +680,9 @@ func TestExtensionWriteBufferedLogsEnrollmentInvalid(t *testing.T) { k.On("Slogger").Return(multislogger.NewNopLogger()) k.On("ReadEnrollSecret").Maybe().Return("enroll_secret", nil) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) e.LogString(context.Background(), logger.LogTypeStatus, "status foo") @@ -1009,7 +1049,9 @@ func TestExtensionGetQueriesTransportError(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) queries, err := e.GetQueries(context.Background()) @@ -1039,7 +1081,9 @@ func TestExtensionGetQueriesEnrollmentInvalid(t *testing.T) { k.On("Slogger").Return(multislogger.NewNopLogger()) k.On("ReadEnrollSecret").Return("enroll_secret", nil) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) e.NodeKey = "bad_node_key" @@ -1067,7 +1111,9 @@ func TestExtensionGetQueries(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) queries, err := e.GetQueries(context.Background()) @@ -1086,7 +1132,9 @@ func TestExtensionWriteResultsTransportError(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) err = e.WriteResults(context.Background(), []distributed.Result{}) @@ -1110,7 +1158,9 @@ func TestExtensionWriteResultsEnrollmentInvalid(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) e.NodeKey = "bad_node_key" @@ -1133,7 +1183,9 @@ func TestExtensionWriteResults(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.Nil(t, err) expectedResults := []distributed.Result{ @@ -1161,7 +1213,9 @@ func TestLauncherRsaKeys(t *testing.T) { k.On("ConfigStore").Return(configStore) k.On("Slogger").Return(multislogger.NewNopLogger()) - _, err = NewExtension(context.TODO(), m, k, ExtensionOpts{}) + _, err = NewExtension(context.TODO(), m, k, ExtensionOpts{ + skipHardwareKeysSetup: true, + }) require.NoError(t, err) key, err := PrivateRSAKeyFromDB(configStore) From bc812099977e3c1a15c6c8b8682b798ac86db7c2 Mon Sep 17 00:00:00 2001 From: james pickett Date: Fri, 8 Mar 2024 11:13:35 -0800 Subject: [PATCH 10/13] more hw key set up skips --- pkg/osquery/extension_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/osquery/extension_test.go b/pkg/osquery/extension_test.go index b99d50e71..07de49f04 100644 --- a/pkg/osquery/extension_test.go +++ b/pkg/osquery/extension_test.go @@ -727,7 +727,8 @@ func TestExtensionWriteBufferedLogsLimit(t *testing.T) { k.On("Slogger").Return(multislogger.NewNopLogger()) e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ - MaxBytesPerBatch: 100, + MaxBytesPerBatch: 100, + skipHardwareKeysSetup: true, }) require.Nil(t, err) @@ -800,7 +801,8 @@ func TestExtensionWriteBufferedLogsDropsBigLog(t *testing.T) { k.On("Slogger").Return(multislogger.NewNopLogger()) e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ - MaxBytesPerBatch: 15, + MaxBytesPerBatch: 15, + skipHardwareKeysSetup: true, }) require.Nil(t, err) @@ -884,9 +886,10 @@ func TestExtensionWriteLogsLoop(t *testing.T) { mockClock := clock.NewMockClock() expectedLoggingInterval := 10 * time.Second e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ - MaxBytesPerBatch: 200, - Clock: mockClock, - LoggingInterval: expectedLoggingInterval, + MaxBytesPerBatch: 200, + Clock: mockClock, + LoggingInterval: expectedLoggingInterval, + skipHardwareKeysSetup: true, }) require.Nil(t, err) @@ -1012,7 +1015,10 @@ func TestExtensionPurgeBufferedLogs(t *testing.T) { k.On("Slogger").Return(multislogger.NewNopLogger()) max := 10 - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{MaxBufferedLogs: max}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + MaxBufferedLogs: max, + skipHardwareKeysSetup: true, + }) require.Nil(t, err) var expectedStatusLogs, expectedResultLogs []string From 5f22b6fbefe4d88d8d9916589a992205d04fefe8 Mon Sep 17 00:00:00 2001 From: james pickett Date: Fri, 8 Mar 2024 11:51:49 -0800 Subject: [PATCH 11/13] more hardware key skipping in tests --- pkg/osquery/log_publication_state_test.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/osquery/log_publication_state_test.go b/pkg/osquery/log_publication_state_test.go index a4f129a77..f00c82532 100644 --- a/pkg/osquery/log_publication_state_test.go +++ b/pkg/osquery/log_publication_state_test.go @@ -23,7 +23,10 @@ func TestExtensionLogPublicationHappyPath(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{MaxBytesPerBatch: startingBatchLimitBytes}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + MaxBytesPerBatch: startingBatchLimitBytes, + skipHardwareKeysSetup: true, + }) require.Nil(t, err) // issue a few successful calls, expect that the batch limit is unchanged from the original opts @@ -57,7 +60,10 @@ func TestExtensionLogPublicationRespondsToNetworkTimeouts(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{MaxBytesPerBatch: startingBatchLimitBytes}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + MaxBytesPerBatch: startingBatchLimitBytes, + skipHardwareKeysSetup: true, + }) require.Nil(t, err) // expect each subsequent failed call to reduce the batch size until the min threshold is reached @@ -106,7 +112,10 @@ func TestExtensionLogPublicationIgnoresNonTimeoutErrors(t *testing.T) { db, cleanup := makeTempDB(t) defer cleanup() k := makeKnapsack(t, db) - e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{MaxBytesPerBatch: startingBatchLimitBytes}) + e, err := NewExtension(context.TODO(), m, k, ExtensionOpts{ + MaxBytesPerBatch: startingBatchLimitBytes, + skipHardwareKeysSetup: true, + }) require.Nil(t, err) // issue a few calls that error immediately, expect that the batch limit is unchanged from the original opts From 721794dd5db7f1095a06baff3a4aaa76ddb8428e Mon Sep 17 00:00:00 2001 From: james pickett Date: Fri, 8 Mar 2024 13:42:43 -0800 Subject: [PATCH 12/13] use available contexts for logging --- ee/secureenclavesigner/secureenclavesigner_darwin.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/secureenclavesigner/secureenclavesigner_darwin.go b/ee/secureenclavesigner/secureenclavesigner_darwin.go index 7dcc1492c..381e804d0 100644 --- a/ee/secureenclavesigner/secureenclavesigner_darwin.go +++ b/ee/secureenclavesigner/secureenclavesigner_darwin.go @@ -59,7 +59,7 @@ func New(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDele if data != nil { if err := json.Unmarshal(data, ses); err != nil { traces.SetError(span, fmt.Errorf("unmarshaling secure enclave signer: %w", err)) - ses.slogger.Log(context.TODO(), slog.LevelError, + ses.slogger.Log(ctx, slog.LevelError, "unable to unmarshal secure enclave signer, data may be corrupt, wiping", "err", err, ) @@ -88,7 +88,7 @@ func New(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDele // get current console user key to make sure it's available if _, err := ses.currentConsoleUserKey(ctx); err != nil { traces.SetError(span, fmt.Errorf("getting current console user key: %w", err)) - ses.slogger.Log(context.TODO(), slog.LevelError, + ses.slogger.Log(ctx, slog.LevelError, "getting current console user key", "err", err, ) From 022537ee956314490eeb485e39ed91ad7f047647 Mon Sep 17 00:00:00 2001 From: james pickett Date: Fri, 8 Mar 2024 14:23:00 -0800 Subject: [PATCH 13/13] dont use hardware keys for signing on darwin --- ee/control/client_http.go | 5 ++++- ee/debug/shipper/shipper.go | 7 ++++++- ee/localserver/krypto-ec-middleware.go | 4 +++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/ee/control/client_http.go b/ee/control/client_http.go index d8b195960..4c163e861 100644 --- a/ee/control/client_http.go +++ b/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) diff --git a/ee/debug/shipper/shipper.go b/ee/debug/shipper/shipper.go index 7dad9584e..41e1ef983 100644 --- a/ee/debug/shipper/shipper.go +++ b/ee/debug/shipper/shipper.go @@ -13,6 +13,7 @@ import ( "net/url" "os" "os/user" + "runtime" "strings" "sync" "time" @@ -206,7 +207,11 @@ func signHttpRequest(req *http.Request, body []byte) { } sign(agent.LocalDbKeys(), control.HeaderKey, control.HeaderSignature, req) - sign(agent.HardwareKeys(), control.HeaderKey2, control.HeaderSignature2, req) + + // hardware signing is not implemented for darwin + if runtime.GOOS != "darwin" { + sign(agent.HardwareKeys(), control.HeaderKey2, control.HeaderSignature2, req) + } } func launcherData(k types.Knapsack, note string) ([]byte, error) { diff --git a/ee/localserver/krypto-ec-middleware.go b/ee/localserver/krypto-ec-middleware.go index 4efe5e604..ca9ed15ee 100644 --- a/ee/localserver/krypto-ec-middleware.go +++ b/ee/localserver/krypto-ec-middleware.go @@ -13,6 +13,7 @@ import ( "log/slog" "net/http" "net/url" + "runtime" "strings" "time" @@ -303,7 +304,8 @@ func (e *kryptoEcMiddleware) Wrap(next http.Handler) http.Handler { // it's possible the keys will be noop keys, then they will error or give nil when crypto.Signer funcs are called // krypto library has a nil check for the object but not the funcs, so if are getting nil from the funcs, just // pass nil to krypto - if e.hardwareSigner != nil && e.hardwareSigner.Public() != nil { + // hardware signing is not implemented for darwin + if runtime.GOOS != "darwin" && e.hardwareSigner != nil && e.hardwareSigner.Public() != nil { response, err = challengeBox.Respond(e.localDbSigner, e.hardwareSigner, bhr.Bytes()) } else { response, err = challengeBox.Respond(e.localDbSigner, nil, bhr.Bytes())